Effective Java : New technique learnt, static factory methods

[Tim Cooke]

Turns out that you can teach an old(ish) dog new tricks!

Item 1: "Consider static factory methods instead of constructors" is something I had not considered before but now have added it as a valuable technique to my programming tool kit. In fact just yesterday I was able to apply the technique to some production code I wrote. Here's what I did.

I started out with a couple of String values being passed around that represented the name and type of a communication transport, it looked a bit like this:

doSomeTransportStuff("ABC123", "TibcoRV");

Can you smell something? That's right, I smell a Data Clump, which is where two or more data items are passed around together because they belong together. The resolution to this smell is to turn those data items into an object. Which is what I did:

doSomeTransportStuff(new Transport("ABC123", "TibcoRV"));

I've now grouped the transport name and type together in an object which is an improvement. But the new Transport constructor doesn't do much to tell us what "ABC123" and "TibcoRV" are and we're now forced to go and have a look at the Transport constructor code in the hope that the parameter names are helpfully named. You might think you could just look at the JavaDoc for this info but for an internal class that is not exposed in any public API it is highly likely there are no comments at all, which is true in this case as I did not write any (on purpose, but that's another topic altogether). Using a static factory method to provide object instantiation can improve the readability of the code in this case, which now looks like:

doSomeTransportStuff(Transport.ofNameAndType("ABC123", "TibcoRV"));

The static factory method provides some context to our previously mystical String values. How does this look in the Transport class?
(Note: the checkNotNull() is a convenience provided by the Google Guava Preconditions library)

public class Transport {
  private String name;
  private String type;

  public Transport(String name, String type) {
    this.name = checkNotNull(name);
    this.type = checkNotNull(type);
  }

  public static Transport ofNameAndType(String name, String type) {
    return new Transport(name, type);
  }

  // Getters for name and type omitted.
}

If you wanted to force the usage of the static factory method for object instantiation then you could make the constructor private. In this case I left it as public because I had no compelling reason to hide it. My next step of course was to represent the type as an enum rather than a String but I'll leave it at that for now as I just wanted to share how you could apply the static factory method in a real life example.

[Campbell Ritchie]

Enhancement to that technique:
If the class is immutable, create a Map<Duple<String, String>, Transport> and you can use it for caching instances.

[Tim Cooke]

Indeed. In which case I would want to make the constructor private to enforce the use of the static factory method.

Where did you get the Duple from in your example? Is it something that's out there or have you written it yourself?

[Campbell Ritchie]

You would have to write your own Duple class, I think. Make it immutable. Not sure how you would take defensive copies of constructor parameters, however.

[Tim Cooke]

Campbell, this has been bugging me when you said

Campbell Ritchie wrote:Not sure how you would take defensive copies of constructor parameters

The constructor parameters are both String so why would you need to make defensive copies of them. Am I missing something?

[Mike Simmons]

The Duple appears to be a parameterized class, so for a Duple<A,B> we might want defensive copies of the A or B instances - if A or B are mutable, and we want the Duple to be immutable. But there's really no good way to do this in general, as there's no standard way to make a copy of an instance of some arbitrary class. Unless we have A and B implement a new interface, or use reflection. In the latter case, we can't really enforce immutability against reflection anyway, so I'm not sure what the point would be. Instead I think it's better to simply say that a Duple<A, B> is immutable if and only if A and B are immutable.

[Campbell Ritchie]

That is what I meant, only MS put it so much better than I did.

[Tim Cooke]

Got it, thanks

[Chan Ag]

Tim Cooke wrote:
I started out with a couple of String values being passed around that represented the name and type of a communication transport, it looked a bit like this:

doSomeTransportStuff("ABC123", "TibcoRV");

Can you smell something? That's right, I smell a Data Clump, which is where two or more data items are passed around together because they belong together. The resolution to this smell is to turn those data items into an object. Which is what I did:

doSomeTransportStuff(new Transport("ABC123", "TibcoRV"));

I've now grouped the transport name and type together in an object which is an improvement.

I think that is an improvement that is sort of understated. I think for APIs that can evolve, using context objects is an indispensable trick.

I would have have also extended the idea to also use a builder to achieve immutability.

Mike Simmons wrote:The Duple appears to be a parameterized class, so for a Duple<A,B> ......

Thanks. You've answered my questions too.

[Tim Cooke]

Chan Ag wrote:I would have have also extended the idea to also use a builder to achieve immutability.

My Transport is immutable. See no setters and no opportunity for a partially constructed object:

public class Transport {
  private String name;
  private String type;

  public Transport(String name, String type) {
    this.name = checkNotNull(name);
    this.type = checkNotNull(type);
  }

  public static Transport ofNameAndType(String name, String type) {
    return new Transport(name, type);
  }

  // Getters for name and type omitted.
}

I'm a big fan of the builder pattern for object construction but I think it only starts looking attractive when you have a large number of constructor parameters. Here I only have 2 params so the static factory method wins for me. But let's take a look anyway.

Object creation using a static factory method:

Transport.ofNameAndType("ABC123", "TibcoRV");

Object creation using the builder pattern:

Transport.Builder.withName("ABC123").withType("TibcoRV").build();

In this case I think the static factory method is tidier. But there's no wrong answer here either.

[Chan Ag]

Tim Cooke wrote:
I'm a big fan of the builder pattern for object construction but I think it only starts looking attractive when you have a large number of constructor parameters. Here I only have 2 params so the static factory method wins for me.

Yeah, that's what I meant also when I said if you have an API that can evolve. This comes handy when after initial development you realize a method needs one more field. If you are provider of an API that is going to be used by many consumers, this is specially helpful. For such cases, if you also desire immutability, you can use a builder to achieve that in an elegant way. And if you need to also cache your immutable objects, you can do that and have a static method dispense the cached objects.

For your case, it seems you have a clear Duple ( at least that is what it looks like to me ).

For your case, it also seems that your code is not relying on immutability of your Transport type objects. So this might not be applicable for your particular case. However, if your objects need to be immutable, you would need to ensure that the runtime class of your Transport type object is Transport only, not a subclass.

I think you would also require the two String fields to be final, even though String(s) are immutable.