General practitioner collection The following code is used to find the usual_gp(General Practitioner) from gpCollection variable of type TreeMap&lt;Long(Date), SummableMap&lt;String(GPId), Integer(GPCount)&gt;&gt; and store result (usual GP) along with date on usualGPCollection Map.

Does this code follow common best practices? Logic implemented on below code is



        Long startDate = gpCollection.firstKey();
        Long endDate = gpCollection.lastKey();

        TreeMap&lt;Long, String&gt; usualGPCollection = new TreeMap&lt;Long, String&gt;();

        for (Long i = startDate; i &lt;= new DateTime(endDate).plusYears(1).getMillis(); i = i + new DateTime(i).plusMonths(1).getMillis()) {

            Long beforeOneYearDate = new DateTime(i).minusMonths(12).getMillis();
            Long beforeSixMonthDate = new DateTime(i).minusMonths(6).getMillis();
            Long beforeThreeMonthDate = new DateTime(i).minusMonths(3).getMillis();

            List&lt;String&gt; returnGPCollection = customSubMap(gpCollection, beforeOneYearDate, i);

            if(returnGPCollection.size() == 1) {

                usualGPCollection.put(i, returnGPCollection.get(0));

            } else if(returnGPCollection.size()&gt;1){

                returnGPCollection = customSubMap(gpCollection, beforeSixMonthDate, i);

                if(returnGPCollection.size() == 1) {

                    usualGPCollection.put(i, returnGPCollection.get(0));

                }else  if(returnGPCollection.size()&gt;1){

                    returnGPCollection = customSubMap(gpCollection, beforeThreeMonthDate, i);

                    if(returnGPCollection.size() == 1) {

                        usualGPCollection.put(i, returnGPCollection.get(0));

                    } else if(returnGPCollection.size()&gt;1){

//                        returnGPCollection = customSubMap(gpCollection, i);//todo

                    }
                }

                returnGPCollection = customSubMap(gpCollection, beforeOneYearDate, i);
            }
    }

// customSubMap() Function is used to find max count GP on given date range.

private List&lt;String&gt; customSubMap(TreeMap&lt;Long, SummableMap&lt;String, Integer&gt;&gt;  gpCollectionMap, Long fromDate, Long toDate) {
        List&lt;String&gt; returnMap = null;

        SortedMap&lt;Long, SummableMap&lt;String, Integer&gt;&gt; temp = gpCollectionMap.subMap(fromDate, toDate);
        Collection values = temp.values();
        Iterator&lt;SummableMap&lt;String, Integer&gt;&gt; test = values.iterator();

        SummableMap&lt;String, Integer&gt; resultMap = new SummableMap&lt;String, Integer&gt;();
        TreeMap&lt;Integer, List&lt;String&gt;&gt; reverseTree = new TreeMap&lt;Integer, List&lt;String&gt;&gt;();

        while (test.hasNext()) {
            resultMap.putAll(test.next());
//            String  key = test.next().clone();
        }

        List mapValues = new ArrayList(resultMap.values());
        Collections.sort(mapValues);

        NavigableMap&lt;String, Integer&gt; resultMapUpdated = sortHashMapByValuesD(resultMap);

        String firstKey = resultMapUpdated.firstKey();
        String secondKey = resultMapUpdated.lowerKey(firstKey);

        Integer firstValue = resultMapUpdated.get(firstKey);
        Integer secondValue = resultMapUpdated.get(secondKey);

        if(firstValue == secondValue){
            returnMap.add(firstKey);
            returnMap.add(secondKey);
        }else {
            returnMap.add(firstKey);
        }

        return returnMap;
    }

    private List&lt;String&gt; customSubMap(TreeMap&lt;Long, SummableMap&lt;String, Integer&gt;&gt;  gpCollectionMap, Long toDate) {

        List&lt;String&gt; returnMap = null;
        Long beforeThreeMonthDate = new DateTime(toDate).minusMonths(3).getMillis();

        SortedMap&lt;Long, SummableMap&lt;String, Integer&gt;&gt; temp = gpCollectionMap.subMap(beforeThreeMonthDate, toDate);
        String latestGP = temp.get(temp.lastKey()).keySet().iterator().next();

        returnMap.add(latestGP);

        return returnMap;
    }
When you have cascading conditions like you have, it can become 'messy'. At some point the design-pattern 'Chain of responsibility' becomes useful....

Consider an interface:

public interface GPSelector {
    String selectGP(TreeMap&lt;Long, SummableMap&lt;String, Integer&gt;&gt; gpCollection, Long calcdate);
}


Then, consider an array of concrete implementations of that interface:

private static final GPSelector[] GPRULES = new GPSelector[] {

    // 1-year rule
    new GPSelector() {
        public String selectGP(TreeMap&lt;Long, SummableMap&lt;String, Integer&gt;&gt; gpCollection, Long calcdate) {
            List&lt;String&gt; lastyear = customSubMap(gpCollection, new DateTime(i).minusMonths(12).getMillis(), calcdate);
            if (lastyear.size() == 1) [
                return lastyear.get(0);
            }
            return null;
        }
    },

    // 6-month rule
    new GPSelector() {
        public String selectGP(TreeMap&lt;Long, SummableMap&lt;String, Integer&gt;&gt; gpCollection, Long calcdate) {
            // return not-null String if there is a successful 6-month candidate....
        }
    },

    .......

};


OK, so now you have an ordered array of rules that progressively select the right UsualGP candidate.

I your code, you can now simply have the following:

public String selectUsualGP(TreeMap&lt;Long, SummableMap&lt;String, Integer&gt;&gt; gpCollection, Long calcdate) {
    for (GPSelector rule : GPRULES) {
        String gp = rule.selectGP(gpCollection, calcdate);
        if (gp != null) {
            return gp;
        }
    }
    return null; // nothing matched.... :(
}


Adding a new rule is easy, just insert it in to the chain at the appropriate place....

I hope the above is enough to show how the chain-of-responsibility pattern can be applied.