Convert Multivalued map to HashMap

Question

I have a Multivalued map (javax.ws.rs.core.MultivaluedMap<String, String>) which I want to convert to regular HashMap so I got below code:

private Map<String, String> convertMultiToRegularMap(MultivaluedMap<String, String> m) {
 Map<String, String> map = new HashMap<String, String>();
 if (m == null) {
 return map;
 }
 for (Entry<String, List<String>> entry : m.entrySet()) {
 String qKey = entry.getKey();
 List<String> values = entry.getValue();
 if (values.size() > 1) {
 String val = "";
 int i = 0;
 for (String s : values) {
 if (i > 0) {
 val += ",";
 }
 val += s;
 i++;
 }
 map.put(qKey, val);
 } else {
 map.put(qKey, values.get(0));
 }
 }
 return map;
}

Is there any improvement I can do here? I am using Java7.

Answer 1

The main part of the code is converting the List<String> value into a String delimited by a comma. Instead of using an external i variable, you can use a StringBuilder and append the comma or not depending on whether it is empty or not.

Regarding your current code:

• Try to avoid concatenating String with +. You should use a StringBuilder when necessary.
• You don't need to make a separate code path for the case where the list is empty in the map value: the regular path handles it also.
• You don't need to store the key and the value in local variables.
• It is indeed a very good idea to return a new empty map instead of null when the incoming map is null.
• If the value stored in the multimap is an empty list, then your code will throw an exception because it tries to access the element 0 when there is no such element.

This would be a proposed code for Java 7:

private Map<String, String> convertMultiToRegularMap(MultivaluedMap<String, String> m) {
 Map<String, String> map = new HashMap<String, String>();
 if (m == null) {
 return map;
 }
 for (Entry<String, List<String>> entry : m.entrySet()) {
 StringBuilder sb = new StringBuilder();
 for (String s : entry.getValue()) {
 if (sb.length() > 0) {
 sb.append(',');
 }
 sb.append(s);
 }
 map.put(entry.getKey(), sb.toString());
 }
 return map;
}

---

As a side-note, when you'll upgrade to Java 8, this code can be made simpler by using the new String.join(delimiter, elements) method.

• Pimgd: One case you are missing: If the old stored value was null, then the new stored value will be empty string. You might want to preserve null in that case. Otherwise, good review.
• Tunaki: @Pimgd Thanks! Actually, if the old value was null, it would throw a NPE at if (values.size() > 1) so I guess there are no null values.
• Pimgd: Would it? I dunno... all I know is that an empty list would throw for values.get(0) with IndexOutOfBoundsException for sure. Depends on how the implementation returns entrySet.
• Tunaki: @Pimgd Yes, if entry.getValue(); returns null then the code throws at the if just after. And you're right, if the list is empty, there is an exception, thanks, I'll edit that in.
• user1950349: @Tunaki Thanks for your suggestion. What's the right way to handle point 5? I don't want this code to throw Exception.