javamultithreadingthread-safetystatic-initializer

Java static factory to create not thread safe object


In the Clean Code book there is an example which I would like to understand better:

public static SimpleDateFormat makeStandardHttpDateFormat() {
    // SimpleDateFormat is not thread safe, so we need to create each instance independently
    SimpleDateFormat df = new SimpleDateFormat("dd/MMM/yyyy:HH:mm:ss Z");
    df.setTimeZone(TimeZone.getTimeZone("GMT"));
    return df
}

I read that SimpleDateFormat is not thread-safe because it: stores intermediate results in instance fields. So if one instance is used by two threads they can mess each other's results. Now I am interested in why using a static factory method is one solution (probably not the best) to avoid thread safety issues with classes like SimpleDateFormat that are not thread-safe?

In case we have two threads A and B, why should it help to make makeStandardHttpDateFormat() a static method? Wouldn't it be the same if makeStandardHttpDateFormat() wasn't static because we create a new instance of SimpleDateFormat anyway for each thread?

The book states that

[...] the comment is perfectly reasonable. It will prevent some overly eager programmer from using a static initializer in the name of efficiency.

Are static methods really that slow? And what does this statement mean? Why should an "overly eager programmer" be stopped from using this static method just because of the comment? The IDE probably wouldn't even display the comment. However, it will probably show that the method is static. So for me the comment makes no sense. At least, not as mentioned in the book.


Solution

  • There isn't anything about the static-ness, or anything about the factory method, that magically makes something thread-safe that otherwise isn't.

    I assume the strategy he's trying to recommend is that every time a piece of code wants a SimpleDateFormat which represents that format, then rather than rely on some instance which is lying around (which may be used by any number of threads), then it should call this factory method instead. i.e. almost every SimpleDateFormat will get used exactly once, then garbage collected.

    I'm not sure why he's contrasting that with a static initializer specifically. I suppose this is what he had in mind. Since multiple threads may access FORMAT, any that do are not thread-safe.

    class Foo {
        private static final SimpleDateFormat FORMAT = new SimpleDateFormat();
    
        static {
            // Uncle Bob disapproves
            format.setTimeZone(TimeZone.getTimeZone("GMT"));
        } 
    }
    

    The factory method is one strategy, but it's not a great one. It ensures that every caller that relies on makeStandardHttpDateFormat as intended will be thread-safe, but it does not guarantee that every caller will rely on makeStandardHttpDateFormat.

    A much better strategy would be to use some immutable variant, like DateTimeFormatter.