11
\$\begingroup\$

This is a statement that I use a lot in my code base:

 (LocalDateTime.now()).format(DateTimeFormatter.ofPattern("ssmmHHddMMMuu"))

Should I create a static method for this or is it overkill?

public static String getTodaysDateInFormat(String format){
  return (LocalDateTime.now()).format(DateTimeFormatter.ofPattern(format));
}
\$\endgroup\$
  • \$\begingroup\$ How/Where are you using this in your code? \$\endgroup\$ – Simon Forsberg Dec 14 '15 at 19:33
  • 1
    \$\begingroup\$ Why do you need to write the current time in so many places? \$\endgroup\$ – CodesInChaos Dec 14 '15 at 21:21
10
\$\begingroup\$

In general I think it's OK to do so, because readability is improved, and DRY principle is encouraged.

But I'd make it a bit more generic not relying on LocalDateTime.now() and pass this as a parameter:

public static String getDateInFormat(DateTime d, String format) {
    return d.format(DateTimeFormatter.ofPattern(format));
}

The call

String formattedDate = getDateInFormat(LocalDateTime.now(),"ssmmHHddMMMuu");

is still shorter, and better readable.


Last but not least try to give that function a name, that expresses more precisely what it does. Something like formatDateToString() maybe.

\$\endgroup\$
  • \$\begingroup\$ I don't really think your getDateInFormat really adds anything, the original code reads just as clear to me. \$\endgroup\$ – Simon Forsberg Dec 14 '15 at 19:44
  • \$\begingroup\$ formatDateToString can even be called formatDate, and oh, look! That's exactly how I read LocalDateTime.now().format(DEFAULT_FORMAT)! \$\endgroup\$ – Simon Forsberg Dec 14 '15 at 19:45
  • \$\begingroup\$ @SimonForsberg Already deleted the default parameter part, and I disagree readability gets better, and writing such is easier. May be the whole question is too opinion based. \$\endgroup\$ – πάντα ῥεῖ Dec 14 '15 at 19:46
  • \$\begingroup\$ @SimonForsberg Also the one decided this is Java code, was TopinFrassi. I've started with the presumption it's c#, but well. \$\endgroup\$ – πάντα ῥεῖ Dec 14 '15 at 19:49
  • \$\begingroup\$ If you use joda-time-2.3-sources.jar then public static String getDateTime(LocalDateTime d, String format) { return d.toString(DateTimeFormat.forPattern(format)); } \$\endgroup\$ – RajKon Dec 15 '15 at 1:50
19
\$\begingroup\$

First of all, the parenthesis around LocalDateTime.now() are useless.

Now, let's see what we're really doing here:

return LocalDateTime.now().format(DateTimeFormatter.ofPattern(format));

We're creating a unique LocalDateTime each time, but I am assuming that you always use the same format, and therefore the DateTimeFormatter.ofPattern(format) object will always be the same.

I would recommend not creating a method for this and instead extract a constant for the format:

private static final DateTimeFormatter FORMAT = DateTimeFormatter.ofPattern("ssmmHHddMMMuu");

And then each time you need it, you just do

LocalDateTime.now().format(FORMAT);

A method named getTodaysDateInFormat provides exactly the same information as the above piece of code, therefore the method does not really provide any additional value.

I don't consider LocalDateTime.now().format(FORMAT) as duplicated code. Or rather: I'm considering just as much code duplication as a call to getTodaysDateInFormat().

\$\endgroup\$
6
\$\begingroup\$

It's not overkill. Don't repeat yourself. Duplicated logic is troublesome. This is not exactly trivial to write, if I found myself writing it twice, I would already extract it to a helper method.

In addition to reducing duplication (which is evil), consider the readability improvement too. The method name is nicely readable, without any thinking. The statement is not too difficult, but not nearly as readable as your nicely descriptive method name.

\$\endgroup\$
6
\$\begingroup\$
(LocalDateTime.now()).format(DateTimeFormatter.ofPattern("ssmmHHddMMMuu"))

The first problem I see in this line of code is that it lacks a big flashing comment explaining why you aren't using an ISO-8601 compliant format.

You should, of course, have this code in a single place. I don't think a static method is quite the right answer though. I'd prefer to see it in an object that helps explain what's going on. LittleEndianTimeEncoder would be better, but it's a bit too implementation specific -- too much what, not enough why (see previous paragraph).

The way you consume this data might help you to understand the why part, which would help with naming. Take a look at the comment explaining why you are using this unusual format -- somewhere in it should be a noun that motivates the format. For instance, if you are serializing time stamps this way because the Wizzobroke service requires it, then the class that does the formatting is probably a WizzobrokeMessageEncoder.

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