2010-01-11 8 views
7

Estoy tratando con un conjunto de objetos de mensaje, cada uno de los cuales tiene un identificador único que les corresponde. Cada mensaje se puede construir desde un Mapa, o desde un ByteBuffer (los mensajes son binarios, pero sabemos cómo transferir desde y hacia una representación binaria).Java: método de fábrica estático y declaraciones de cambio

La implementación actual para la construcción de estos mensajes es aproximadamente el siguiente:

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

Ahora, Josh Bloch's Effective Java habla de Punto 1: Considere métodos de fábrica estáticas en lugar de constructores, y esto parece ser un lugar donde este patrón es útil (los clientes no tienen acceso directo a los constructores de los subtipos de mensajes, sino que pasan por este método). Pero no me gusta el hecho de que tengamos que recordar mantener dos instrucciones de cambio actualizadas (viola el principio DRY).

Agradecería cualquier idea sobre la mejor manera de lograr esto; no estamos almacenando objetos en caché (cada llamada a fromMap o fromByteBuffer devolverá un nuevo objeto), lo que niega algunos de los beneficios de utilizar un método de fábrica estático como este. Algo acerca de este código me parece erróneo, por lo que me encantaría escuchar las opiniones de la comunidad sobre si esta es una forma válida de construir nuevos objetos, o si no sería una mejor solución.

Respuesta

12

tal vez se podría crear una MessageFactory interfaz y las implementaciones de la misma:

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

} 

siguiente, un método getFactoryFromId en la misma clase que los métodos anteriores:

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

Sin embargo, en lugar de esto, es mejor crear un Hashmap que contenga los identificadores y las fábricas, de modo que no tenga que crear un nuevo objeto Factory cada vez que esté creando un mensaje. Vea también el comment below.

y sus métodos:

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

De esta manera, se utiliza el patrón de la fábrica, y no hay necesidad de tener dos veces la misma sentencia switch.

(no probar esto, así que posiblemente algunos errores de compilación/errores tipográficos)

+0

Si los objetos de fábrica se almacenan en un mapa y se recuperan con uuid, no es necesario cambiarlos. – rsp

+0

yup, eso es lo que dije :-) Incluso agregué un enlace a tu respuesta :) – Fortega

+0

Es una buena solución (así que +1 para eso) pero si el objetivo era deshacerte del doble cambio, podrías haber tenido en cuenta fuera de la lógica del interruptor a un método separado. (el mapa es básicamente un interruptor disfrazado) – wds

0

podría utilizar el patrón de AbstractFactory, donde tendría una clase de fábrica para cada tipo de mensaje, que se devuelve el mensaje, ya sea por búfer o mapa. A continuación, crea un método que le devuelve el objeto de fábrica correspondiente. De la fábrica devuelta, entonces creas el mensaje.

1

¿Hay alguna manera de convertir el ByteBuffer en un mapa u otra cosa? Sería bueno si convierte la entrada a una forma normalizada y aplica un interruptor único.

Si lo que quieres hacer es recibir un mensaje y formatearlo con valores específicos (como "La tabla: tableName no tiene una columna llamada: colName") puedes convertir el ByteBuffer en un mapa y llamar al primero método. Si necesita un nuevo msgId, amplía solo el método fromMap.

Es algo así como factorizar la parte común.

3

Si usted tiene sus objetos implementan una interfaz declarar métodos de fábrica como:

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

en una clase anidada estática, su fábrica puede crear un Map que contienen objetos de fábrica de un índice por el UUID:

Map map = new HashMap(); 

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

y reemplace sus interruptores por búsqueda en el mapa:

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

esto se puede ampliar fácilmente para admitir otros métodos de construcción.

+0

Solo un pequeño comentario: Deberías usar Integer.valueOf() en vez de un nuevo Integer(). – I82Much

+0

Buen punto, cambió eso. – rsp

1

que recomiendan el uso de un tipo de enumeración con los métodos abstractos, como el siguiente ejemplo:

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

} 

De esta manera, en sus métodos estáticos existentes que simplemente puede hacer esto ...

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

Este enfoque evita que su método estático tenga que conocer los tipos de mensajes que admite y cómo compilarlos: puede agregar, modificar o eliminar mensajes sin refactorizar los métodos estáticos.

0

Usted debe abstracta su FirstMessage objeto:

public abstract Message { 
    // ... 
} 

Entonces almacena en caché en su fábrica (en contraposición a un interruptor):

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

En el método de fábrica:

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

Eso es solo una idea de todos modos, tendrías que hacer algunas reflexiones (obtener el constructor) para pasar los campos.

0

Puede modificar Message para que tenga dos métodos de inicialización, uno para Map y otro para ByteBuffer (en lugar de las dos versiones de contructor). Luego, su método de fábrica devuelve el mensaje construido (pero no inicializado), luego llama al inicializador con el mapa o ByteBuffer en el objeto devuelto.

por lo que ahora tienen un método de fábrica de esta manera: -

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

} 

y luego los métodos de fábrica público se convierten en: -

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

y

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

tem 1: Considere métodos de fábrica estáticos en lugar de constructores

que estás haciendo que ya ocultando el constructor detrás de ese método de fábrica, así que no hay necesidad de añadir otro método de fábrica aquí.

Para que pueda hacerlo con una interfaz de fábrica y un mapa.(Básicamente lo que todo el mundo está diciendo ya, pero con la diferencia, que puede inline las fábricas usando clases internas)

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

Y sus invocaciones se convertirá en:

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

Debido a que el UUID utilizado para el el interruptor se usa como llave para las fábricas.

0

Acerca de la solución usando la enumeración como estrategia (agregue métodos de estrategia a una enumeración) la aplicación de hoja de cheet de código limpio dice que se trata de un asesino de mantenimiento.
Aunque no sé por qué me gustaría compartir esto con usted.

+0

Interesante. Pero por favor agregue comentarios como estos como comentarios en lugar de respuestas. – I82Much

Cuestiones relacionadas