Java - static factory method and switch statements

Question

I am dealing with a set of message objects, each of which has a unique identifier corresponding to them. Each message can be constructed either from a Map, or from a ByteBuffer (the messages are binary, but we know how to transfer to and from a binary representation).

The current implementation for constructing these messages is roughly as follows:

public static Message fromMap(int uuid, Map<String, Object> fields) {
    switch (uuid) {
      case FIRST_MESSAGE_ID:
        return new FirstMessage(fields);
        .
        .
        .
      default:
          // Error
          return null;
    }
}

public static Message fromByteBuffer(int uuid, ByteBuffer buffer) {
    switch (uuid) {
      case FIRST_MESSAGE_ID:
        return new FirstMessage(buffer);
        .
        .
        .
      default:
          // Error
          return null;
    }
}

Now, Josh Bloch's Effective Java talks about Item 1: Consider static factory methods instead of constructors, and this seems to be a place where this pattern is useful (clients don't directly access the constructors of the Message subtypes; instead they go through this method). But I do not like the fact that we have to remember to keep two switch statements updated (violates the DRY principle).

I would appreciate any insight into the best way to accomplish this; we're not caching objects (each call to fromMap or fromByteBuffer will return a new object), which negates some of the benefit of using a static factory method like this. Something about this code strikes me as wrong, so I would love to hear the community's thoughts on whether this is a valid way to construct new objects, or if not what a better solution would be.

Answer 1

Maybe you could create an interface MessageFactory and implementations of it:

public interface MessageFactory {
   Message createMessage(Map<String, Object> fields);
   Message createMessage(ByteBuffer buffer);
}

public class FirstMessageFactory implements MessageFactory {
  public Message createMessage(Map<String, Object> fields){
    return new FirstMessage(fields);
  }

  public Message createMessage(ByteBuffer buffer){
    return new FirstMessage(buffer);
  }

}

next, a method getFactoryFromId in the same class as the methods above:

public static MessageFactory getMessageFactoryFromId(int uuid){
 switch (uuid) {
  case FIRST_MESSAGE_ID:
    return new FirstMessageFactory();
    ...
  default:
      // Error
      return null;
  }
}

However, instead of this, it is better to create a Hashmap containing the ids and the factories, so you don't have to create a new Factory object everytime you are creating a message. See also the comment below.

and your methods:

public static Message fromMap(int uuid, Map<String, Object> fields)  {
  getMessageFactoryFromId(uuid).createMessage(fields);
}

public static Message fromByteBuffer(int uuid, ByteBuffer buffer) {
  getMessageFactoryFromId(uuid).createMessage(buffer);
}

This way, you are using the factory pattern, and there is no need to have two times the same switch statement.

(didn't test this, so possibly some compile-errors/typos)

Answer 2

tem 1: Consider static factory methods instead of constructors

You're doing that already by hiding the constructor behind that factory method, so there is no need to add another factory method here.

So you can do it with a Factory interface and a map. ( Basically what everyone is saying already, but with the difference, that you can inline the factories using inner classes )

interface MessageFactory {
    public Message createWithMap( Map<String,Object> fields );
    public Message createWithBuffer( ByteBuffer buffer );
}

Map<MessageFactory> factoriesMap = new HashMap<MessageFactory>() {{
    put( FIRST_UUID, new MessageFactory() {
        public Message createWithMap( Map<String, Object> fields ) {
            return new FirstMessage( fields );
        }
        public Message createWithBuffer( ByteBuffer buffer ){ 
            return new FirstMessage( buffer );
        }
    } );
    put( SECOND_UUID, new MessageFactory(){
        public Message createWithMap( Map<String, Object> fields ) {
            return new SecondMessage( fields );
        }
        public Message createWithBuffer( ByteBuffer buffer ){ 
            return new SecondMessage( buffer );
        } 
    } );
    put( THIRD_UUID, new MessageFactory(){
        public Message createWithMap( Map<String, Object> fields ) {
            return new ThirdMessage( fields );
        }
        public Message createWithBuffer( ByteBuffer buffer ){ 
            return new ThirdMessage( buffer );
        } 
    } );
    ...
}};

And your invocations will turn into:

public static Message fromMap(int uuid, Map<String, Object> fields) {
    return YourClassName.factoriesMap.get( uuid ).createWithMap( fields );
}

public static Message fromByteBuffer(int uuid, ByteBuffer buffer) {
    return YourClassName.factoriesMap.get(uuid).createWithBuffer( buffer );
}

Because the uuid used for the switch is used as key for the factories.

Answer 3

If you have your objects implement an interface declaring factory methods like:

public Message newInstance(Map<String, Object> fields);
public Message newInstance(ByteBuffer buffer);

in a static nested class, your factory can create a Map containing factory objects indexed by the uuid:

Map map = new HashMap();

map.put(Integer.valueOf(FirstMessage.UUID), new FirstMessage.Factory());

and replace your switches by map lookup:

public static Message fromMap(int uuid, Map<String, Object> fields) {

    Factory fact = map.get(Integer.valueOf(uuid));

    return (null == fact) ? null : fact.newInstance(fields);
}

public static Message fromByteBuffer(int uuid, ByteBuffer buffer) {

    Factory fact = map.get(Integer.valueOf(uuid));

    return (null == fact) ? null : fact.newInstance(buffer);
}

this can easily be extended to support other construction methods.