Java - static factory method and switch statements

Question

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

The current implementation for building these messages is approximately the following:

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 Efficient Java talks about point 1: consider static factory methods instead of constructors, and this seems to be where this template is useful (clients do not have direct access to message subtype constructors, they pass this method instead). But I do not like the fact that we must remember to update two switch statements (violates the DRY principle).

I would appreciate a better way to achieve this; we don’t cache objects (every call from fromMap or fromByteBuffer will return a new object), which negates some of the benefits of using a static factory method like this. Something in this code seems wrong to me, so I would like to hear the community’s thoughts about whether this is the right way to create new objects or if not the best solution.

Answer 1

Perhaps you could create a MessageFactory interface and its implementation:

 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, the getFactoryFromId method 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, it is better to create a Hashmap containing identifiers and factories, so you do not need to create a new Factory object each time you create a message. See also 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); } 

Thus, you are using the Factory pattern, and there is no need to double the same switch statement.

(not tested this, maybe some compilation errors / typos)

Answer 2

If you have objects that implement an interface that declares factory methods, for example:

 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 UUID:

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

and replace the switches with a map search:

 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); } 

it can be easily expanded to support other construction methods.

Answer 3

tem 1: consider static factory methods instead of constructors

You already do this by hiding the constructor behind this factory method, so there is no need to add another factory method here.

So you can do this using the factory interface and map. (Basically, everyone says it already, but with the difference that you can embed 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 calls 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 uuid used for switch is used as a key for factories.

Answer 4

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

If you want to receive a message and format it with specific values ​​(for example, "Table: tableName does not have a column with the name: colName"), you can convert ByteBuffer to Map and call the first method. If you need a new msgId, you only distribute the fromMap method .

This is a bit like factoring the common part.

Answer 5

I recommend using an enumeration type with abstract methods, for example, 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); } 

So in your existing static methods, you can just 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 eliminates the need for your static method to know about the MessageTypes you support and how to create them — you can add, modify, or delete messages without refactoring the static methods.

Answer 6

You can use the AbstractFactory template, where you will have a factory class for each type of message that returns a message to you with either a buffer or a map. Then you create a method that returns you the corresponding factory object. From the returned factory you will create a message.

Answer 7

You must abstract your FirstMessage object:

 public abstract Message { // ... } 

Then cache them in the factory (as opposed to the 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(); } 

This is just an idea anyway, you will need to do some thought (get a constructor) to pass the fields.

Answer 8

You can modify Message to have two initialization methods: one for Map and one for ByteBuffer (instead of two versions of contructor). Then your factory method returns a constructed (but uninitialized) message, then you call initialization using Map or ByteBuffer on the returned object.

So now you have a factory method, for example:

 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 will 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 9

About the solution using enum as a strategy (add strategy methods to the listing), the clean-text cheet cheet application says it's a killer for maintenance.
Although I do not know why I would like to share this with you.