5
\$\begingroup\$

I write my log messages in JTextArea. Each message should have a time added. I created a following method:

public void log(String message) {
    logTextArea.append(String.format("%s : %s%n",DateFormatUtils.ISO_TIME_NO_T_FORMAT.format(new Date()), message));
}

It uses DateFormatUtils from Apache Commons Lang. Each time log() method is invoked, it creates a new instance of Date.

Is this the most elegant method?

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

Elegant and efficient are different things.

SimpleDateFormat instances are not thread safe. It is common in my code to have a few utility classes, and one method I often have on the utility class is:

private static final ThreadLocal<SimpleDateFormat> localDateFormat = new ThreadLocal<SimpleDateFormat>() {

    @Override
    protected SimpleDateFormat initialValue() {
             return new SimpleDateFormat(....);
    }

};

public static SimpleDateFormat getDateFormatter() {
    return localDateFormat.get();
}

That way, anyone calling getDateFormatter() gets an instance of the standard formatter for my system, and it is local to the current thread. I don't let them 'hang around', so you can't assign them to class fields, etc.

The benefit is that there's at most one instance per thread, it is thread safe (if you don't do silly things with it), and it is neat:

public void log(String message) {
    logTextArea.append(String.format("%s : %s%n",
           Utils.getDateFormat().format(new Date()), message));
}

Unfortunately, I don't think you have taken this far enough. You need another utility method like:

public static String formatLogMessage(String message) {
    return String.format("%s : %s%n",
           Utils.getDateFormat().format(new Date()), message);
}

and then all your various places that modify things like this can reuse the code:

public void log(String message) {
    logTextArea.append(Utils.formatLogMesssage(message));
}

Update: I somewhat misread your specific question, and in response to your comment:

  1. creating a new short-term new Date() is not perfect, but is still very fast (it will be cycled out by the garbage collector very quickly, and not impact that very much at all). There are perhaps better ways though...

  2. I don't use the commons library, so was not aware that the FastDateFormat class was thread-safe (it probably does what I suggest above), but, at the same time, the extra log-method is a good idea because it brings all the different log places in to one spot so you can change formats, etc. with just one changed line of code.

Additionally, if you are concerned about the actual performance of the new Date(), (and if you actually read the full Javadoc of the FastDateFormat class you sent me... ;-) you can replace that one method:

public static String formatLogMessage(String message) {
    return String.format("%s : %s%n",
           Utils.getDateFormat().format(new Date()), message);
}

with:

public static String formatLogMessage(String message) {
    return String.format("%s : %s%n",
           DateFormatUtils.ISO_TIME_NO_T_FORMAT.format(System.currentTimeMillis()), message);
}
\$\endgroup\$
2
  • \$\begingroup\$ Actually, ISO_TIME_NO_T_FORMAT is a FastDateFormat, which is a thread-safe version of SimpleDateFormat as mentioned in documentation \$\endgroup\$ – Kao Oct 23 '14 at 11:48
  • \$\begingroup\$ Added update (follow the link near the bottom). \$\endgroup\$ – rolfl Oct 23 '14 at 12:00

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.