Get start and end dates and strings

Question

I have a method to get the time parameter, is this code neat, can the return parameter use other data structures?

private Map<String, Object> getTimeCondition(long smallDaysToSubtract, long bigDaysToSubtract) {
 DateTimeFormatter format = DateTimeFormatter.ofPattern(DateUtil.yyyy_MM_dd);
 ZoneId zoneId = ZoneId.systemDefault();
 LocalDate startLocalDate = LocalDate.now().minusDays(smallDaysToSubtract);
 LocalDate endLocalDate = LocalDate.now().minusDays(bigDaysToSubtract);
 LocalDateTime startLocalDateTime = LocalDateTime.of(startLocalDate, LocalTime.MIN);
 LocalDateTime endLocalDateTime = LocalDateTime.of(endLocalDate, LocalTime.MAX);
 Instant startInstant = startLocalDateTime.atZone(zoneId).toInstant();
 Instant endInstant = endLocalDateTime.atZone(zoneId).toInstant();
 Date startTime = Date.from(startInstant);
 Date endTime = Date.from(endInstant);
 String startDateStr = startLocalDate.format(format);
 String endDateStr = endLocalDate.format(format);
 Map<String, Object> map = new HashMap<>();
 map.put("startTime", startTime);
 map.put("endTime", endTime);
 map.put("startDateStr", startDateStr);
 map.put("endDateStr", endDateStr);
 return map;
}

• Phrancis: Welcome to Code Review! I hope you get some great answers.

Answer 1

Hmm... I cannot really make much of this code - what is the pupose of that parameter map?

However, there are a few things to note:

• You make use of the modern datetime-api, which is great! And you use it in a correct way - I cannot see any flaws there. BUT in the end you throw all that precision, zoning and niceness overboard and convert everything into old.ugly.Date - why? As long as you use this internally, you should aim for the more precise and powerful api and only convert if necessary for legacy code.
• Mashup of different tasks: in most cases, the formatting of data (or dates in this case) belongs to the presentation of an application, whereas the precise values belong to the business logic. Thus, I'd leave the formatting out of the method and move this into the presentation layer, which would also help with the next point...:
• Map<String, Object> as a result value. With Object values you throw away all generic goodness a map has, and have to cast the values when you use them. This could be circumvented by eiher heeding my advice in the point above (returning Map<String, LocalDateTime>) or by creating a result class which just consists of the four fields with the correct data types.
• Magic strings: "startTime", "endTime", etc. should probably be moved to publicly accessible constants, so the the consumer of the map can use these constants to look up the values he needs without "knowing" and duplicating the concrete key-strings.

• dai: Wow, thank you for your suggestion.Can you elaborate on your fourth suggestion?
• mtj: Simply declare something like public static final String START_TIME = "startTime"; in your class, so that whoever uses this can write map.get(YourClass.START_TIME) instead of map.get("startTime"). That way, you can change the concrete string at any time and no knowledge of internal values is needed to use the class.