2
\$\begingroup\$

I have a project requirement where I have to split a HashMap of around 60-70 entries into multiple HashMaps. The critical number of the split is 30 due to performance reasons because eventually this HashMap is going to be wrapped in a customer Request object and sent in an API call. The receiving end during performance testing found that 30 is the only number they can take for good performance.

So, I have to split the HashMap of 70 subscribers into sub-HashMaps of - 30, 30, 10.

Since HashMaps do not have methods like - subMap() or tailMap() - I am thinking of converting the HashMap into TreeMap. Here's my implementation:

checkAndSplitSubscribers(Map<String, Type> originalMap) throws Exception { List<Map<String, Type>> listOfSplitMaps = new ArrayList<Map<String, Type>>(); int criticalNumber = 30; (Although this will be configurable) try { TreepMap<String, Type> treeMap = new TreeMap<String, Type> (originalMap); List<String> keys = new ArrayList<String> (originalMap.keySet()); final int originalMapSize = treeMap.size(); for (int i = 0; i < originalMapSize; i += criticalNumber) { if (i + criticalNumber < originalMapSize) { listOfSplitMaps.add(treeMap.subMap(keys.get(i), keys.get(i + criticalNumber)); } else { listOfSplitMaps.add(treeMap.tailMap(keys.get(i)); } } } catch (Exception e) { throw e; } return listOfSplitMaps; } 

I want to ask if there is anything wrong with this implementation? Is converting HashMap to TreeMap mid way of the code really a bad programming practice? Or if there's any better way to achieve the above, please suggest (I have searched and gone through almost all splitting hashmap questions on SO but I did not find any answer to my satisfaction).

\$\endgroup\$
1
  • \$\begingroup\$Instead of creating a TreeMap you could just iterate over the map and add elements to map's in a list\$\endgroup\$
    – Mibac
    CommentedJul 15, 2017 at 21:10

2 Answers 2

2
\$\begingroup\$

Interfaces over implementations

 TreepMap<String, Type> treeMap = new TreeMap<String, Type> (originalMap); 

This could be

 SortedMap<String, Type> splittable = new TreeMap<>(original); 

That gets rid of the typo TreepMap.

It saves specifying the types twice. You don't need to do that unless you are building against an old version of Java.

Specifying the interface rather than the implementation makes it easier to change implementations in the future. You could also use NavigableMap, but it's not necessary for these operations.

I prefer descriptive names like splittable and original to type names. Of course, your coding standards could be different.

Exceptions

 } catch (Exception e) { throw e; } 

If you're just going to throw the same exception, there's no reason to catch it. If you want to catch it, then handle it. If you want it to percolate up to the caller, then just let it go without catching it. As is, the try/catch doesn't do anything. You could simplify the code by taking it out.

Alternatives

I'm not convinced that this does enough to be worthwhile. It's probably going to be less efficient than the more manual solution. And it's not less code. Consider

 Map<String, Type> current = new HashMap<>(MAXIMUM_CAPACITY); for (Map.Entry<String, Type> entry : original.entrySet()) { current.put(entry.getKey(), entry.getValue()); if (current.size() >= MAXIMUM_SIZE) { splitMaps.add(current); current = new HashMap<>(MAXIMUM_CAPACITY); } } if (!current.isEmpty()) { splitMaps.add(current); } 

This is about the same amount of code, but it saves turning the keySet into a List.

Both versions have to iterate over the original map. This one does so manually while the original code did so implicitly by converting to a TreeMap.

I would find the code more compelling if the data was moved from the HashMap to a TreeMap as the base source. I.e. if there never was a HashMap and the map came as a TreeMap.

This assumes the creation of two new constants. You might be able to find better names with more context. I don't like criticalNumber as a name because it doesn't tell me what makes the number critical. MAXIMUM_SIZE is a bit better in my opinion, as it tells me that the number is a size and that we don't want to go beyond it.

\$\endgroup\$
    0
    \$\begingroup\$

    OK Mibac. Here's what I did and I think you are right TreeMap is not needed. I just wanted to use the TreeMap's subMap/tailMap methods.

     public List<Map<String, String>> split(Map<String, String> original) { int max = 5; int counter = 0; int lcounter = 0; List<Map<String, String>> listOfSplitMaps = new ArrayList<Map<String, String>> (); Map<String, String> splitMap = new HashMap<> (); for (Map.Entry<String, String> m : original.entrySet()) { if (counter < max) { splitMap.put(m.getKey(), m.getValue()); counter++; lcounter++; if (counter == max || lcounter == original.size()) { counter = 0; listOfSplitMaps.add(splitMap); splitMap = new HashMap<> (); } } } return listOfSplitMaps; } 
    \$\endgroup\$

      Start asking to get answers

      Find the answer to your question by asking.

      Ask question

      Explore related questions

      See similar questions with these tags.