0

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.

evolved
  • 1,850
  • 19
  • 40
  • The best way to avoid threading problems with `SimpleDateFormat` _specifically_ is to not use `java.util.Date` at all and to use the modern `java.time` instead. – chrylis -cautiouslyoptimistic- Jul 18 '22 at 23:47
  • Thanks for the hint. I understand that `SimpleDateFormat` isn't the best method to use when dealing with multi threading and we should use other methods like you mention. But I would like to understand the specific example of the book better. – evolved Jul 18 '22 at 23:50
  • The code above doesn't make SimpleDateFormat threadsafe. Two threads having a reference to the same SimpleDateFormat object would cause problem if they concurrently modifying it. I imagine that author meant to say each thread has its own copy of a SimpleDateFormat object. – Cheng Thao Jul 19 '22 at 03:54

1 Answers1

5

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.

Michael
  • 41,989
  • 11
  • 82
  • 128
  • Also, my opinion, clean code is not a very good book. Read it with skepticism. This post gives a good account of it: https://qntm.org/clean – Michael Jul 19 '22 at 00:20
  • Thanks for the answer. As I am not really acquainted with Java could you please elaborate a little bit more on your example that Uncle Bob most likely meant? How could two threads use class Foo and result in thread safety issues? – evolved Jul 19 '22 at 19:49
  • @evolved SimpleDateFormat is mutable. One thread could mutate it somehow while another thread is in the process of formatting a date to a string. If you change the format, while the format is being applied, you open yourself up to all kinds of weird inconsistencies and exceptions. – Michael Jul 20 '22 at 10:57