Java - static factory method and switch statements

Question

Hi guys,

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

You could use the AbstractFactory pattern, where you would have a factory class for each message type, that returns you the message either by buffer or map. You then create a method that returns you the corresponding factory object. From the returned factory you then create the message.

Answer 3

Is there a way to convert the ByteBuffer to a Map or something else? It would be nice if you convert the input to a normalized form and apply an unique switch.

If what you want to do is getting a message and formatting it with specific values (like "The table :tableName doesn't have a column named :colName") you can convert the ByteBuffer to a Map and calling the first method. If you need a new msgId you extend only the fromMap method.

It's something like factoring the common part.

Answer 4

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.

Answer 5

I recommend using an enum type with abstract methods, such as the following example:

enum MessageType {

    FIRST_TYPE(FIRST_MESSAGE_ID) {

        @Override
        Message fromByteBuffer(ByteBuffer buffer) {
            return new FirstMessage(buffer);
        }

        @Override
        Message fromMap(Map<String, Object> fields) {
            return new FirstMessage(fields);
        }

        @Override
        boolean appliesTo(int uuid) {
            return this.uuid == uuid;
        }

    },

    SECOND_TYPE(SECOND_MESSAGE_ID) {

        @Override
        Message fromByteBuffer(ByteBuffer buffer) {
            return new SecondMessage(buffer);
        }

        @Override
        Message fromMap(Map<String, Object> fields) {
            return new SecondMessage(fields);
        }

        @Override
        boolean appliesTo(int uuid) {
            return this.uuid == uuid;
        }

    };

    protected final int uuid;

    MessageType(int uuid) {
        this.uuid = uuid;
    }

    abstract boolean appliesTo(int uuid);

    abstract Message fromMap(Map<String, Object> map);

    abstract Message fromByteBuffer(ByteBuffer buffer);

}

This way, in your existing static methods you can simply do this...

public static Message fromByteBuffer(int uuid, ByteBuffer buffer) {
    Message rslt = null;
    for (MessageType y : MessageType.values()) {
        if (y.appliesTo(uuid)) {
            rslt = y.fromByteBuffer(buffer);
            break;
        }
    }
    return rslt;
}

This approach saves your static method from having to know about the MessageTypes you support and how to build them -- you can add, modify, or remove messages without refactoring the static methods.

Answer 6

You should abstract your FirstMessage object:

public abstract Message {
   // ...
}

Then cache them in your factory (as opposed to a switch):

private static final Map<Integer, Class<Message>> MESSAGES = new HashMap<Integer, Class<Message>>();
static {
    MESSAGES.put(1, FirstMessage.class);
}

In your factory method:

public static Message fromMap(UUID uuid, Map<String, Object> fields) {
    return MESSAGES.get(uuid).newInstance();
}

That's just an idea anyway, you'd have to do some reflections (get the constructor) work to pass the fields.

Answer 7

You could modify Message so that it has two initialize methods, one for Map and one for ByteBuffer (instead of the two contructor versions). Then your factory method returns the constructed (but uninitialized) Message, then you call initialize with the Map or ByteBuffer on the returned object.

So you now have a factory method like this:-

private static Message createMessage(int uuid) {
    switch (uuid) {
      case FIRST_MESSAGE_ID:
        return new FirstMessage();
        .
        .
        .
      default:
          // Error
          return null;
    }

}

and then the public factory methods become:-

public static Message fromMap(int uuid, Map<String, Object> fields) {
  Message message = createMessage(uuid);
  // TODO: null checking etc....
  return message.initialize(fields);
}

and

public static Message fromByteBuffer(int uuid, ByteBuffer buffer) {
  Message message = createMessage(uuid);
  // TODO: null checking etc....
  return message.initialize(buffer);
}

Answer 8

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.