2
\$\begingroup\$

I have a String which is coming in particular format so I want to parse that string into map. I have a below method which parses String to a Map and it works fine.

  public static Map<String, String> parseStringToMap(String payload) {
    Map<String, String> map = new HashMap<>();
    try {
      for (String part : payload.split("\\|")) {
        String[] subparts = part.split("=", 2);
        map.put(subparts[0], subparts[1]);
      }
    } catch (Exception ex) {
    }
    return map;
  }

Sample example of a string:

"type=3|Id=23456|user=13456"
"type=3|Id=23456|user=13456|type=3"

It might be possible that same key can appear many times in the same string payload so I need to overwrite the value for that key in my mutable map.

Also if my string payload is not in this format then I would like to return empty map back. My string format will always be like this and if it is not in this format then I would return empty map back.

"a=b|c=d|e=f"   

I am still working with Java 7. What is the best and efficient way to do this?

\$\endgroup\$
  • \$\begingroup\$ Maybe if(subparts.length!=2){return Collections.emptyMap();} (writing this from my mobile please do not copy/paste that.) \$\endgroup\$ – Roman Gräf Sep 21 '17 at 21:21
1
\$\begingroup\$

You can not get it much more efficient that this. It could be more readable and there are some code smells.

Remarks:

  • The input string "zoo=ooz=10" has a map with 1 entry that has "ooz" as value. Is this what you want? Maybe not limit the split and check that it is 2?
  • "catch (Exception ex)" is considered bad practice. You are basically saying that you do not care about any Exceptions. Also any possible exceptions in all the methods you call. This is never a good idea.
  • No argument checking. The cases where the argument is null or the empty string you can leave the method early. This improves readability. Now the massive catch also does the argument checking. Having a dedicated piece of code for every thing is easier to read.
  • You are transforming one iterable (result of payload.split) into another dynamic sized iterable. Usually it is better to then set the initial capacity for the HashMap to prevent rehashes during insertion which is O(n). But in this case the end size does not have to be equal to the input size. It is a border case.

My proposal:

public static Map<String, String> parseStringToMap(String payload) {
  Map<String, String> map = new HashMap<>();

  if (payload == null || payload.isEmpty()) {
    return map;
  }

  for (String part : payload.split("\\|")) {
      String[] subparts = part.split("=");
      if (subparts.length == 2) {
        map.put(subparts[0], subparts[1]);
      }
  }


  return map;
}
\$\endgroup\$
0
\$\begingroup\$

This is a fair implementation.

But given an input of "type=3|Id=23456|user=13456|foo" you will wind up returning a three-element map. That doesn't appear to match your English specification.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.