Refactorización de código Java método de fábrica
https://stackoverflow.com/questions/115269
italiano
english
français
española
中国
日本の
العربية
Deutsch
한국어
Português
RussianPregunta
Hay algo muy insatisfactorio acerca de este código:
/*
Given a command string in which the first 8 characters are the command name
padded on the right with whitespace, construct the appropriate kind of
Command object.
*/
public class CommandFactory {
public Command getCommand(String cmd) {
cmdName = cmd.subString(0,8).trim();
if(cmdName.equals("START")) {
return new StartCommand(cmd);
}
if(cmdName.equals("END")) {
return new EndCommand(cmd);
}
// ... more commands in more if blocks here
// else it's a bad command.
return new InvalidCommand(cmd);
}
}
Estoy arrepentido acerca de los múltiples puntos de salida - la estructura es clara.Pero no estoy contento con la serie de casi idénticas, si las declaraciones.He pensado en hacer un Mapa de Cadenas de Comandos:
commandMap = new HashMap();
commandMap.put("START",StartCommand.class);
// ... etc.
...a continuación, el uso de la Reflexión para hacer que las instancias de la clase apropiada levantó la vista del Mapa.Sin embargo, aunque conceptualmente elegante, esto implica una buena cantidad de código de Reflexión que quien hereda este código podría no apreciar - a pesar de que los costos podrían ser compensados por los beneficios.Todas las líneas de codificar los valores en la commandMap olor casi tan malo como si el bloque.
Sería aún mejor si la fábrica del constructor podría escanear el classpath para subclases de Mando, consultar para la Cadena de representaciones, y las añadirá a su repertorio.
Pues, ¿cómo debo ir sobre refactorización esto?
Supongo que algunos de los marcos de ahí me dan este tipo de cosas gratis.Supongamos que yo no estoy en una posición para migrar eso en un marco de.
Solución
El mapa de cadenas de comandos yo creo que es bueno.Incluso se podría factor de la cadena de comando nombre del constructor (es decir,no StartCommand sé que su mandamiento es "INICIO"?) Si usted puede hacer esto, la creación de instancias de su comando de los objetos es mucho más simple:
Class c = commandMap.get(cmdName);
if (c != null)
return c.newInstance();
else
throw new IllegalArgumentException(cmdName + " is not as valid command");
Otra opción es crear un enum de todos los comandos con los enlaces a las clases (asumiendo que todos sus objetos de comando implementar CommandInterface):
public enum Command
{
START(StartCommand.class),
END(EndCommand.class);
private Class<? extends CommandInterface> mappedClass;
private Command(Class<? extends CommandInterface> c) { mappedClass = c; }
public CommandInterface getInstance()
{
return mappedClass.newInstance();
}
}
desde el toString de una enumeración es su nombre, se puede utilizar EnumSet para localizar el objeto correcto y obtener la clase desde dentro.
Otros consejos
Cómo sobre el siguiente código:
public enum CommandFactory {
START {
@Override
Command create(String cmd) {
return new StartCommand(cmd);
}
},
END {
@Override
Command create(String cmd) {
return new EndCommand(cmd);
}
};
abstract Command create(String cmd);
public static Command getCommand(String cmd) {
String cmdName = cmd.substring(0, 8).trim();
CommandFactory factory;
try {
factory = valueOf(cmdName);
}
catch (IllegalArgumentException e) {
return new InvalidCommand(cmd);
}
return factory.create(cmd);
}
}
El valueOf(String) de la enumeración se utiliza para encontrar el correcto método de fábrica.Si la fábrica no existe, se producirá una IllegalArgumentException.Podemos utilizar esto como una señal para crear la InvalidCommand objeto.
Un beneficio extra es que si usted puede hacer que el método create(String cmd) público, si quisieras hacer este camino de construcción de un objeto de Comando de tiempo de compilación comprobada disponible para el resto de su código.Usted podría utilizar CommandFactory.START.create(String cmd) para crear un objeto de Comando.
La última ventaja es que usted puede crear fácilmente una lista de todos los comandos disponibles en la documentación Javadoc.
Con la excepción de la
cmd.subString(0,8).trim();
en parte, esto no parece demasiado malo para mí.Usted podría ir con el Mapa y el uso de la reflexión, pero, dependiendo de la frecuencia de agregar/cambiar los comandos, esto podría no comprar mucho.
Probablemente, usted debe documentar por qué desea que sólo los 8 primeros caracteres, o tal vez cambiar el protocolo para que sea más fácil para averiguar qué parte de esa cadena es el comando (por ejemplo,poner un marcador como ':' o ';' después de que el comando de la palabra clave).
No es directamente una respuesta a su pregunta, pero ¿por qué no lanzar una InvalidCommandException (o algo similar), en lugar de devolver un objeto de tipo InvalidCommand?
A menos que exista una razón que no puede ser siempre trato de hacer mi comando implementaciones de apátridas.Si ese es el caso, usted puede agregar un método booleano identificador(String id) método para la interfaz de comandos que dicen si esta instancia podría ser utilizado para la cadena de identificador.Luego de su fábrica podría ser algo como esto (nota:Yo no compilar o prueba de esto):
public class CommandFactory {
private static List<Command> commands = new ArrayList<Command>();
public static void registerCommand(Command cmd) {
commands.add(cmd);
}
public Command getCommand(String cmd) {
for(Command instance : commands) {
if(instance.identifier(cmd)) {
return cmd;
}
}
throw new CommandNotRegisteredException(cmd);
}
}
Me gusta tu idea, pero si quieres evitar el reflejo usted podría agregar en lugar de instancias para el HashMap:
commandMap = new HashMap();
commandMap.put("START",new StartCommand());
Siempre que usted necesite un comando, que acaba de clonar:
command = ((Command) commandMap.get(cmdName)).clone();
Y después, se establece la cadena de comando:
command.setCommandString(cmdName);
Pero el uso de clone() no suena tan elegante como el uso de la reflexión :(
Tomando un Convetion vs enfoque de Configuración y el uso de la reflexión para buscar objetos de Comando y la carga en el mapa sería el camino a seguir.A continuación, tiene la capacidad de exponer nuevos Comandos sin una recompilación de la fábrica.
Otro enfoque de la dinámica encontrar la clase a la carga, sería omitir explícito en el mapa, y tratar de construir el nombre de la clase de la cadena de comando.Un caso de título y concatenar algoritmo podría convertirse en "INICIO" -> "com.mypackage.los comandos.StartCommand", y sólo tiene que utilizar la reflexión para tratar de crear instancias de ella.Fallar de alguna manera (InvalidCommand instancia o una Excepción de su propia) si usted no puede encontrar la clase.
A continuación, puede agregar comandos con sólo agregar un objeto y empezar a usarlo.
Sería una opción para cada tipo de comando a tener su propia fábrica.Esto le da dos ventajas:
1) Tu genérico de fábrica no llamaría de nuevo.Para cada tipo de comando en el futuro podrían devolver un objeto de una clase diferente de acuerdo con los argumentos siguientes el espacio de relleno en la cadena.
2) En el HashMap esquema, podría evitar la reflexión, para cada clase de comando, la asignación a un objeto de la implementación de un SpecialisedCommandFactory interfaz, en lugar de la asignación a la misma clase.Este objeto, en la práctica, probablemente sería un singleton, pero no necesita ser especificado como tal.Su genérico getCommand, a continuación, llama a la especializada getCommand.
Dicho esto, la fábrica de proliferación puede salir de la mano, y el código es la cosa más simple que hace el trabajo.Personalmente creo que probablemente iba a dejarlo como está:usted puede comparar comando muestra en el código fuente y especificaciones sin consideraciones locales como lo que podría haber llamado previamente CommandFactory.registerCommand, o qué clases se han descubierto a través de la reflexión.No es confuso.Es muy poco probable que sea lento para menos de un millar de comandos.El único problema es que usted no puede agregar nuevos tipos de comandos sin necesidad de modificar la fábrica.Pero la modificación que haría es simple y repetitivo, y si usted se olvida de hacer que se obtiene un error obvio de líneas de comandos que contiene el nuevo tipo, así que no es oneroso.
Tener esta repetitivo de creación de objetos de código oculto en la fábrica no es tan malo.Si se tiene que hacer en algún lugar, al menos todo está aquí, así que me gustaría que no se preocupe demasiado.
Si realmente quiero hacer algo al respecto, tal vez ir por el Mapa, pero configurarlo desde un archivo de propiedades, y construir el mapa de ese archivo props.
Sin ir a la ruta de clases de descubrimiento de ruta (sobre la cual no conozco), siempre vas a ser la modificación de 2 lugares:escribir una clase y, a continuación, agregar una asignación en algún lugar (de fábrica, mapa init, o archivo de propiedades).
Pensando en esto, Se podría crear un poco de la creación de instancias de clases, como:
class CreateStartCommands implements CommandCreator {
public bool is_fitting_commandstring(String identifier) {
return identifier == "START"
}
public Startcommand create_instance(cmd) {
return StartCommand(cmd);
}
}
Por supuesto, esto añade un montón si pequeñas clases que no pueden hacer mucho más que decir "sí, eso es empezar, a mi me da que" o "no, no me gusta eso", sin embargo, ahora se puede rediseñar la fábrica para contener una lista de los CommandCreators y simplemente hacer que cada uno de:"te gusta este comando?" y devolver el resultado de create_instance de la primera aceptación de CommandCreator.Por supuesto que ahora se ve un poco incómodas para la extracción de los primeros 8 caracteres fuera de la CommandCreator, por lo que sería de renovación que así pase toda la cadena de comando en el CommandCreator.
Creo que he aplicado algunos "Reemplazar el interruptor con el polimorfismo"-Refactorización aquí, en caso de que alguien se pregunta acerca de eso.
Yo iría a por el mapa y la creación a través de la reflexión.Si la exploración de la ruta de clase es demasiado lento, siempre se puede añadir una anotación personalizadas para la clase, tiene una anotación procesador se ejecuta en tiempo de compilación y almacenar todos los nombres de clase en el frasco de metadatos.
Entonces, el único error que puede hacer es olvidarse de la anotación.
Yo hice algo como esto hace un tiempo, el uso de maven y APT.
La manera en que lo hago es para no tener un genérico método de Fábrica.
Me gusta usar los Objetos de Dominio como mis objetos de comando.Desde que tengo uso de Spring MVC este es un gran enfoque, ya que el DataBinder.setAllowedFields método me permite una gran flexibilidad para el uso de un único dominio objeto de varias formas diferentes.
Para obtener un objeto de comando, tengo un estático de fábrica método en el objeto de Dominio de clase.Por ejemplo, en los miembros de la clase me han métodos como el de -
public static Member getCommandObjectForRegistration();
public static Member getCommandObjectForChangePassword();
Y así sucesivamente.
No estoy seguro de que este es un gran enfoque, yo nunca lo vi sugerido en cualquier lugar y tipo de acaba de llegar con mi cuenta b/c me gusta la idea de mantener este tipo de cosas en un solo lugar.Si alguien ve ninguna razón para oponerse, por favor hágamelo saber en los comentarios...
Sugiero evitar la reflexión, si es posible.Es algo malo.
Usted puede hacer su código más concisa mediante el operador ternario:
return
cmdName.equals("START") ? new StartCommand (cmd) :
cmdName.equals("END" ) ? new EndCommand (cmd) :
new InvalidCommand(cmd);
Podría introducir una enumeración.Hacer de cada enumeración constante de una fábrica es detallado y también tiene una memoria de tiempo de ejecución costo.Pero usted puede eaily búsqueda de una enumeración y, a continuación, utilizar ese con == o interruptor.
import xx.example.Command.*;
Command command = Command.valueOf(commandStr);
return
command == START ? new StartCommand (commandLine) :
command == END ? new EndCommand (commandLine) :
new InvalidCommand(commandLine);
Siga su instinto, y reflexionar.Sin embargo, en esta solución, su interfaz de Comandos ahora se supone que tienen la setCommandString(String s) método accesible, por lo que newInstance es fácilmente utilizable.También, commandMap es cualquier mapa con claves de tipo Cadena (cmd) al Comando de la clase de las instancias que corresponden.
public class CommandFactory {
public Command getCommand(String cmd) {
if(cmd == null) {
return new InvalidCommand(cmd);
}
Class commandClass = (Class) commandMap.get(cmd);
if(commandClass == null) {
return new InvalidCommand(cmd);
}
try {
Command newCommand = (Command) commandClass.newInstance();
newCommand.setCommandString(cmd);
return newCommand;
}
catch(Exception e) {
return new InvalidCommand(cmd);
}
}
Hmm, la navegación, y sólo acaba de llegar a través de este.Todavía me puede comentar?
En mi humilde opinión hay nada mal con el original if/else bloque de código.Esto es simple, y la simplicidad debe ser siempre la primera llamada en el diseño (http://c2.com/cgi/wiki?DoTheSimplestThingThatCouldPossiblyWork)
Esto parece esp cierto, como todas las soluciones que ofrece son mucho menos la documentación de que el código original...me refiero a que no debemos escribir nuestro código para la lectura en lugar de la traducción...
Al menos, el comando debe tener un getCommandString() -- donde StartCommand reemplaza a regresar "INICIO".Entonces usted puede registrarse o descubrir las clases.
+1 en la reflexión de la sugerencia, se le dará una más sana estructura en su clase.
En realidad se podría hacer lo siguiente (si no lo has pensado ya) crear métodos correspondientes a la Cadena que estaría esperando como un argumento a su getCommand() método de fábrica, entonces todo lo que tienes que hacer es reflexionar y invoke() estos métodos y devuelve el objeto correcto.
