3
\$\begingroup\$

Consider I have a String:

String value = "Sunday  -  H15,Sunday  -  H03,Sunday  -  H13,Sunday  -  H01,Sunday  -  H05,Friday  -  H23,Saturday  -  H05,Monday  -  H16,Monday  -  H17,Monday  -  H18,Monday  -  H19,Monday  -  H20";

In this String we have Days (Sunday, Monday etc..) and the alphanumeric (H15, H03 etc..) is hours.

Now I want a JSON format for Map<String, Map<String, List<String>>> in which Key should be Day and Value should be the Map having Key as Hours and Values as list of Hours which has same day. Something like this format:

{"Monday":{"Hours":["H16","H17","H18","H19","H20"]},"Thursday":{"Hours":[]},"Friday":{"Hours":["H23"]},"Sunday":{"Hours":["H15","H03","H13","H01","H05"]},"Wednesday":{"Hours":[]},"Tuesday":{"Hours":[]},"Saturday":{"Hours":["H05"]}}

Note: In this example there are some days with valid hours (like Sunday, Monday etc) while other are having empty Hours (like Wednesday and Thursday).

This code is working fine as required but I feel that it can be refactored.

public class DayPartProcessingTest {

  public static void main(String[] args) throws JsonProcessingException {
    String propertyValue = "Sunday  -  H15,Sunday  -  H03,Sunday  -  H13,Sunday  -  H01,Sunday  -  H05,Friday  -  H23,Saturday  -  H05,Monday  -  H16,Monday  -  H17,Monday  -  H18,Monday  -  H19,Monday  -  H20";
    Map<String, List<String>> map = new HashMap<String, List<String>>();
    String[] dayParts = propertyValue.split(",");

    for (String string : dayParts) {
      String[] array = string.split("  -  ");
      if (map.get(array[0]) == null) {
        List<String> list = new ArrayList<>();
        list.add(array[1]);
        map.put(array[0], list);
      } else {
        List<String> tempList = map.get(array[0]);
        tempList.add(array[1]);
        map.put(array[0], tempList);
      }
    }

    Map<String, Map<String, List<String>>> payload = new HashMap<>();
    Map<String, List<String>> hrs1 = new HashMap<>();
    hrs1.put("Hours", new ArrayList<String>());
    payload.put("Sunday", hrs1);
    payload.put("Monday", hrs1);
    payload.put("Tuesday", hrs1);
    payload.put("Wednesday", hrs1);
    payload.put("Thursday", hrs1);
    payload.put("Friday", hrs1);
    payload.put("Saturday", hrs1);
    for (Entry<String, List<String>> entry : map.entrySet()) {
      Map<String, List<String>> hrs = new HashMap<>();
      hrs.put("Hours", entry.getValue());
      if (payload.containsKey(entry.getKey())) {
        payload.put(entry.getKey(), hrs);
      }
    }

    String json = new ObjectMapper().writeValueAsString(payload);
    System.out.println(json);

  }
\$\endgroup\$
  • 1
    \$\begingroup\$ Can't you just have a Map<String, List<String>>? There's no point in having a map with a single key. \$\endgroup\$ – Solomon Ucko Aug 19 '17 at 17:49
1
\$\begingroup\$

Methods

The first thing that we can do is divide up the labor. Consider

void static main(String[] args) throws JsonProcessingException {
    Map<String, List<String>> properties = parseProperties(getPropertyValue());
    Map<String, Map<String, List<String>>> payload = buildPayload(properties);
    String json = new ObjectMapper().writeValueAsString(payload);
    System.out.println(json);
}

Now we have the heavy lifting in parseProperties and buildPayload.

I also hid the propertyValue in a method, but that was mostly to avoid obnoxious horizontal scrolling.

parseProperties

    String[] dayParts = propertyValue.split(",");

    for (String string : dayParts) {
      String[] array = string.split("  -  ");
      if (map.get(array[0]) == null) {
        List<String> list = new ArrayList<>();
        list.add(array[1]);
        map.put(array[0], list);
      } else {
        List<String> tempList = map.get(array[0]);
        tempList.add(array[1]);
        map.put(array[0], tempList);
      }
    }

Consider

    for (String string : propertyValue.split(",")) {
      String[] parts = string.split("  -  ");

      List<String> hours = map.get(parts[0]);
      if (hours == null) {
        hours = new ArrayList<>();
      }

      hours.add(parts[1]);
      map.put(parts[0], hours);
    }

We don't need a separate variable for the results of the comma split. We only use that for this loop, so we can do it directly.

We do need a separate variable for the parts of that string, as we use the parts separately. Also, we use one of them multiple times.

In both branches of the original if, we do two of the same commands. If we rewrite it this way, we can do those outside the if.

In the original if, we could have used contains, as we don't use the value otherwise. But this way, we save the result. If it is empty, we put an empty list there. Otherwise, we can just use the previous value.

buildPayload

    Map<String, Map<String, List<String>>> payload = new HashMap<>();
    Map<String, List<String>> hrs1 = new HashMap<>();
    hrs1.put("Hours", new ArrayList<String>());
    payload.put("Sunday", hrs1);
    payload.put("Monday", hrs1);
    payload.put("Tuesday", hrs1);
    payload.put("Wednesday", hrs1);
    payload.put("Thursday", hrs1);
    payload.put("Friday", hrs1);
    payload.put("Saturday", hrs1);
    for (Entry<String, List<String>> entry : map.entrySet()) {
      Map<String, List<String>> hrs = new HashMap<>();
      hrs.put("Hours", entry.getValue());
      if (payload.containsKey(entry.getKey())) {
        payload.put(entry.getKey(), hrs);
      }
    }

So you want to create a map with the seven days of the week. The same pattern can work here.

public static Map<String, List<String>> normalizeHours(Map<String, List<String>> hours) {
    if (hours == null) {
        hours = new HashMap<>();
        hours.put("Hours", new ArrayList<String>());
    }

    return hours;
}

And we'd use it like

    Map<String, Map<String, List<String>>> payload = new HashMap<>();
    payload.put("Sunday", normalizeHours(properties.get("Sunday")));
    payload.put("Monday", normalizeHours(properties.get("Monday")));
    payload.put("Tuesday", normalizeHours(properties.get("Tuesday")));
    payload.put("Wednesday", normalizeHours(properties.get("Wednesday")));
    payload.put("Thursday", normalizeHours(properties.get("Thursday")));
    payload.put("Friday", normalizeHours(properties.get("Friday")));
    payload.put("Saturday", normalizeHours(properties.get("Saturday")));

Instead of putting a dummy value into each day of the week and then replacing it, this puts the correct value in in the first place.

It's not shorter but is more reusable to have a container for the days of the week that you can use here.

    for (String dayOfTheWeek : dayNames) {
        payload.put(dayOfTheWeek, normalizeHours(properties.get(dayOfTheWeek)));
    }

It's not much longer even for this single problem. Once you have the container, it shortens the code.

\$\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.