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));
}
-
\$\begingroup\$ How/Where are you using this in your code? \$\endgroup\$Simon Forsberg– Simon Forsberg2015年12月14日 19:33:01 +00:00Commented Dec 14, 2015 at 19:33
-
1\$\begingroup\$ Why do you need to write the current time in so many places? \$\endgroup\$CodesInChaos– CodesInChaos2015年12月14日 21:21:14 +00:00Commented Dec 14, 2015 at 21:21
4 Answers 4
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.
-
\$\begingroup\$ I don't really think your
getDateInFormat
really adds anything, the original code reads just as clear to me. \$\endgroup\$Simon Forsberg– Simon Forsberg2015年12月14日 19:44:22 +00:00Commented Dec 14, 2015 at 19:44 -
\$\begingroup\$
formatDateToString
can even be calledformatDate
, and oh, look! That's exactly how I readLocalDateTime.now().format(DEFAULT_FORMAT)
! \$\endgroup\$Simon Forsberg– Simon Forsberg2015年12月14日 19:45:45 +00:00Commented Dec 14, 2015 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\$πάντα ῥεῖ– πάντα ῥεῖ2015年12月14日 19:46:43 +00:00Commented Dec 14, 2015 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\$πάντα ῥεῖ– πάντα ῥεῖ2015年12月14日 19:49:29 +00:00Commented Dec 14, 2015 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– RajKon2015年12月15日 01:50:56 +00:00Commented Dec 15, 2015 at 1:50
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()
.
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.
(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
.
Explore related questions
See similar questions with these tags.