Call to method of static java.text.DateFormat not advisable?

I am receiving a Find Bugs error - call to method of static java.text.DateFormat and I don't know the reason why it's not good / advisable to be doing these things below.

private static final Date TODAY = Calendar.getInstance().getTime();
private static final DateFormat yymmdd = new SimpleDateFor开发者_高级运维mat("yyMMdd"); 

private String fileName = "file_" + yymmdd.format(TODAY);

---

DateFormats are not thread-safe, meaning that they maintain an internal representation of state. Using them in a static context can yield some pretty strange bugs if multiple threads access the same instance simultaneously.

My suggestion would be to make your variables local to where you're using them instead of making them static properties of the class. It looks like you might be doing this when you're initializing the class, so you could do this in the constructor:

public class MyClass {
    private String fileName;

    public MyClass() {
        final Date today = Calendar.getInstance().getTime();
        final DateFormat yymmdd = new SimpleDateFormat("yyMMdd"); 

        this.fileName = "file_" + yymmdd.format(TODAY);
    }
    ...
}

And if you need to use the formatter in multiple places, you might just make the pattern static final and create a new local DateFormat when needed:

public class MyClass {
    private static final String FILENAME_DATE_PATTERN = "yyMMdd";

    public void myMethod() {
        final DateFormat format = new SimpleDateFormat(FILENAME_DATE_PATTERN);
        // do some formatting
    }
}

The FindBugs documentation for the issue says:

As the JavaDoc states, DateFormats are inherently unsafe for multithreaded use. The detector has found a call to an instance of DateFormat that has been obtained via a static field. This looks suspicous.

For more information on this see Sun Bug #6231579 and Sun Bug #6178997.

And the javadoc for DateFormat suggests:

Date formats are not synchronized. It is recommended to create separate format instances for each thread. If multiple threads access a format concurrently, it must be synchronized externally.

Jack Leow's answer also has a good point about the semantics of your static use of "TODAY".

As an aside, I've actually seen this happen in a high-traffic production environment, and it's a very confusing thing to debug at first; so in my experience the FindBugs warning is actually a useful suggestion (unlike some other static analysis rules, which sometimes seem to be nitpicky).

---

Commons Lang has a FastDateFormat object that is thread safe. It only does formatting though, not parsing.

If you can use commons-lang this might work well for you.

private static final Date TODAY = Calendar.getInstance().getTime();
private static final FastDateFormat yymmdd = FastDateFormat.getInstance("yyMMdd");

private String fileName = "file_" + yymmdd.format(TODAY);

---

An alternative that not has been mentioned is using ThreadLocal. See http://www.javacodegeeks.com/2010/07/java-best-practices-dateformat-in.html for more info + performance comparison between the 3 options:

• Creating an instance each time
• Synchronising access
• Using ThreadLocal

Example of using ThreadLocal:

private static final ThreadLocal<SimpleDateFormat> DATE_FORMAT = new ThreadLocal<SimpleDateFormat>() {
    @Override
    protected SimpleDateFormat initialValue() {
        return new SimpleDateFormat("yyMMdd");
    }
};

Usage:

DATE_FORMAT.get().format( TODAY )

---

Are you sure it's not

private static final DateFormat yymmdd = new SimpleDateFormat("yyMMdd"); 

? That's what the error message indicates.

I think what it aims at is the fact that DateFormat is not thread safe, so having an instance as a static field indicates potential race conditions.

---

I'm not sure if FindBugs is complaining about this, but one problem I see with your code is that you're defining TODAY as a class level (static), constant (final) variable. This communicates the intent that you want TODAY to never change (I don't believe this is the case, since java.util.Dates are mutable, but that's another story).

Think about what happens if you application runs for multiple days? TODAY (unless you update it) will be referencing the day the application was started, not the current date. Are you sure this is what you meant?

This may not be a bug in your code at all, but the intent is not clear, and I believe that may be what FindBugs is complaining about.

---

It's not thread-safe, for one thing.

---

I assume this is because format is not thread-safe?

(I have not seen what findbugs complain about, can you provide the warning text?)

---

You can get this to go away by wrapping all the references to the DateFormat in a synchronize block - just make sure all calls are wrapped in the same synchronize object!