The following code is used to find the usual_gp(General Practitioner) from gpCollection variable of type TreeMap<Long(Date), SummableMap<String(GPId), Integer(GPCount)>> 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<Long, String> usualGPCollection = new TreeMap<Long, String>();        for (Long i = startDate; i <= 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<String> returnGPCollection = customSubMap(gpCollection, beforeOneYearDate, i);            if(returnGPCollection.size() == 1) {                usualGPCollection.put(i, returnGPCollection.get(0));            } else if(returnGPCollection.size()>1){                returnGPCollection = customSubMap(gpCollection, beforeSixMonthDate, i);                if(returnGPCollection.size() == 1) {                    usualGPCollection.put(i, returnGPCollection.get(0));                }else  if(returnGPCollection.size()>1){                    returnGPCollection = customSubMap(gpCollection, beforeThreeMonthDate, i);                    if(returnGPCollection.size() == 1) {                        usualGPCollection.put(i, returnGPCollection.get(0));                    } else if(returnGPCollection.size()>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<String> customSubMap(TreeMap<Long, SummableMap<String, Integer>>  gpCollectionMap, Long fromDate, Long toDate) {        List<String> returnMap = null;        SortedMap<Long, SummableMap<String, Integer>> temp = gpCollectionMap.subMap(fromDate, toDate);        Collection values = temp.values();        Iterator<SummableMap<String, Integer>> test = values.iterator();        SummableMap<String, Integer> resultMap = new SummableMap<String, Integer>();        TreeMap<Integer, List<String>> reverseTree = new TreeMap<Integer, List<String>>();        while (test.hasNext()) {            resultMap.putAll(test.next());//            String  key = test.next().clone();        }        List mapValues = new ArrayList(resultMap.values());        Collections.sort(mapValues);        NavigableMap<String, Integer> 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<String> customSubMap(TreeMap<Long, SummableMap<String, Integer>>  gpCollectionMap, Long toDate) {        List<String> returnMap = null;        Long beforeThreeMonthDate = new DateTime(toDate).minusMonths(3).getMillis();        SortedMap<Long, SummableMap<String, Integer>> temp = gpCollectionMap.subMap(beforeThreeMonthDate, toDate);        String latestGP = temp.get(temp.lastKey()).keySet().iterator().next();        returnMap.add(latestGP);        return returnMap;    }
General practitioner collection
java;optimization
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<Long, SummableMap<String, Integer>> 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<Long, SummableMap<String, Integer>> gpCollection, Long calcdate) {            List<String> 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<Long, SummableMap<String, Integer>> 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<Long, SummableMap<String, Integer>> 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.