93

Tener una cadena de operaciones "instanceof" se considera un "olor a código". La respuesta estándar es "usar polimorfismo". ¿Cómo lo haría en este caso?Evitar instanceof en Java

Existen varias subclases de una clase base; ninguno de ellos está bajo mi control. Una situación análoga sería con las clases de Java Integer, Double, etc. BigDecimal

if (obj instanceof Integer) {NumberStuff.handle((Integer)obj);} 
else if (obj instanceof BigDecimal) {BigDecimalStuff.handle((BigDecimal)obj);} 
else if (obj instanceof Double) {DoubleStuff.handle((Double)obj);} 

tengo control sobre NumberStuff y así sucesivamente.

No quiero usar muchas líneas de código donde algunas líneas harían. (A veces hago un mapeo HashMap Integer.class a una instancia de IntegerStuff, BigDecimal.class a una instancia de BigDecimalStuff etc. Pero hoy quiero algo más simple.)

me gustaría algo tan simple como esto:

public static handle(Integer num) { ... } 
public static handle(BigDecimal num) { ... } 

Pero Java simplemente no funciona de esa manera.

Me gustaría utilizar métodos estáticos al formatear. Las cosas que estoy formateando son compuestas, donde un Thing1 puede contener un array Thing2s y un Thing2 pueden contener un conjunto de Thing1s. Tenía un problema cuando he implementado mis formateadores de la siguiente manera:

class Thing1Formatter { 
    private static Thing2Formatter thing2Formatter = new Thing2Formatter(); 
    public format(Thing thing) { 
     thing2Formatter.format(thing.innerThing2); 
    } 
} 
class Thing2Formatter { 
    private static Thing1Formatter thing1Formatter = new Thing1Formatter(); 
    public format(Thing2 thing) { 
     thing1Formatter.format(thing.innerThing1); 
    } 
} 

Sí, sé que el HashMap y un poco más de código puede arreglar eso también. Pero el "instanceof" parece tan legible y mantenible en comparación. ¿Hay algo simple pero no huele mal?

Nota añadida 5/10/2010:

Resulta que las nuevas subclases probablemente se añadirán en el futuro, y mi código existente tendrá que manejarlas con garbo. El HashMap on Class no funcionará en ese caso porque no se encontrará la clase. Una cadena de si las declaraciones, empezando por la más específica y terminando con el más general, es probablemente el mejor, después de todo:

if (obj instanceof SubClass1) { 
    // Handle all the methods and properties of SubClass1 
} else if (obj instanceof SubClass2) { 
    // Handle all the methods and properties of SubClass2 
} else if (obj instanceof Interface3) { 
    // Unknown class but it implements Interface3 
    // so handle those methods and properties 
} else if (obj instanceof Interface4) { 
    // likewise. May want to also handle case of 
    // object that implements both interfaces. 
} else { 
    // New (unknown) subclass; do what I can with the base class 
} 
+4

Sugeriría un [patrón de visitante] [1]. [1]: http://en.wikipedia.org/wiki/Visitor_pattern – lexicore

+0

¿Es la cadena ifthen a la que se opone, o simplemente el uso de "instanceof"? – Greg

+20

El patrón Visitor requiere agregar un método a la clase objetivo (Integer por ejemplo) - fácil en JavaScript, difícil en Java. Excelente patrón al diseñar las clases objetivo; no es tan fácil cuando intentamos enseñarle a un viejo Clase nuevos trucos. –

Respuesta

47

que podría estar interesado en esta entrada de Amazon el blog de Steve Yegge: "when polymorphism fails". Esencialmente se está dirigiendo a casos como este, cuando el polimorfismo causa más problemas de los que resuelve.

El problema es que para usar el polimorfismo debe hacer que la lógica de "manejar" sea parte de cada clase 'de conmutación', es decir, Integer, etc. en este caso. Claramente esto no es práctico. A veces ni siquiera es lógicamente el lugar correcto para poner el código. Él recomienda el enfoque 'instancia de' como el menor de varios males.

Como en todos los casos en que se ve obligado a escribir un código oloroso, manténgalo abrochado en un método (o como máximo en una clase) para que el olor no se filtre.

+18

El polimorfismo no falla. Por el contrario, Steve Yegge no puede inventar el patrón Visitor, que es el reemplazo perfecto para 'instanceof'. – Rotsor

+8

No veo cómo ayuda el visitante aquí. El punto es que la respuesta de OpinionatedElf a NewMonster no debe codificarse en NewMonster, sino en OpinionatedElf. – DJClayworth

+1

Si 'Monster' se hace visitable, el duende puede visitarlo para extraer toda la información que necesita, manteniendo así la lógica en la clase' OpinionatedElf' sin recurrir a las prácticas peligrosas de usar la reflexión. – Rotsor

20

Como se destaca en los comentarios, el patrón de visitante sería una buena opción. Pero sin control directo sobre el objetivo/aceptor/visitee, no puede implementar ese patrón. He aquí una forma en que el patrón de visitante podría todavía ser usada aquí a pesar de que usted no tiene control directo sobre las subclases mediante el uso de envolturas (teniendo entero como un ejemplo):

public class IntegerWrapper { 
    private Integer integer; 
    public IntegerWrapper(Integer anInteger){ 
     integer = anInteger; 
    } 
    //Access the integer directly such as 
    public Integer getInteger() { return integer; } 
    //or method passthrough... 
    public int intValue() { return integer.intValue(); } 
    //then implement your visitor: 
    public void accept(NumericVisitor visitor) { 
     visitor.visit(this); 
    } 
} 

Por supuesto, envolviendo una clase final que podría considerarse un olor propio, pero tal vez es un buen ajuste con sus subclases.Personalmente, no creo que instanceof sea un mal olor aquí, especialmente si está limitado a un método y lo usaría felizmente (probablemente por encima de mi propia sugerencia). Como dices, es bastante legible, seguro y fácil de mantener. Como siempre, mantenlo simple.

+0

Sí," Formatter "," Composite "," Different types "todas las costuras para apuntar la dirección del visitante. –

+2

¿cómo se determina qué envoltorio va a utilizar? a través de un if instancia de ramificación? –

+1

Como @fasttooth señala esta solución, solo cambia el problema. En lugar de usar 'instanceof' para llamar al método' handle() 'correcto, ahora tendrá que usar la llamada del constructor' XWrapper' correcto ... – Matthias

13

En lugar de un gran if, puede poner los casos en que usted maneja en un mapa (clave: clase, valor: handler).

Si la búsqueda mediante la tecla devuelve null, llame a un método de controlador especial que intenta encontrar un controlador coincidente (por ejemplo, llamando al isInstance() en cada tecla del mapa).

Cuando se encuentra un controlador, registrarlo en la nueva clave.

Esto hace que el caso general rápido y sencillo y le permite manejar la herencia.

+0

+1 He usado este enfoque al manejar código generado a partir de esquemas XML , o sistema de mensajería, donde hay docenas de tipos de objetos, entregados a mi código de una manera esencialmente no segura. – DNA

9

Usted podría considerar the Chain of Responsibility pattern. Para su primer ejemplo, algo como:

public abstract class StuffHandler { 
    private StuffHandler next; 

    public final boolean handle(Object o) { 
     boolean handled = doHandle(o); 
     if (handled) { return true; } 
     else if (next == null) { return false; } 
     else { return next.handle(o); } 
    } 

    public void setNext(StuffHandler next) { this.next = next; } 

    protected abstract boolean doHandle(Object o); 
} 

public class IntegerHandler extends StuffHandler { 
    @Override 
    protected boolean doHandle(Object o) { 
     if (!o instanceof Integer) { 
     return false; 
     } 
     NumberHandler.handle((Integer) o); 
     return true; 
    } 
} 

y luego de manera similar para sus otros controladores. Entonces, es un caso de agrupar los StuffHandlers en orden (de más específico a menos específico, con un manejador final de 'respaldo'), y su código de despachador es solo firstHandler.handle(o);.

(Una alternativa es, en lugar de usar una cadena, simplemente tener un List<StuffHandler> en su clase de despachador, y hacer que recorra la lista hasta que handle() devuelva verdadero).

13

Puede utilizar la reflexión:

public final class Handler { 
    public static void handle(Object o) { 
    try { 
     Method handler = Handler.class.getMethod("handle", o.getClass()); 
     handler.invoke(null, o); 
    } catch (Exception e) { 
     throw new RuntimeException(e); 
    } 
    } 
    public static void handle(Integer num) { /* ... */ } 
    public static void handle(BigDecimal num) { /* ... */ } 
    // to handle new types, just add more handle methods... 
} 

pueden ampliar en la idea de manejar de forma genérica subclases y clases que implementan ciertas interfaces.

+26

Yo diría que esto huele incluso más que el operador de instanceof. Debería funcionar sin embargo. –

+3

@Tim Büthe: Al menos no tiene que lidiar con una creciente cadena 'if then else' para agregar, eliminar o modificar manejadores. El código es menos frágil a los cambios. Entonces, diría que, por esta razón, es superior al enfoque 'instanceof'. De todos modos, solo quería dar una alternativa válida. –

+0

es suficiente, upvote. –

9

Creo que la mejor solución es HashMap con Class como clave y Handler como valor. Tenga en cuenta que la solución basada en HashMap se ejecuta en complejidad algorítmica constante θ (1), mientras que la cadena olorosa de if-instanceof-else se ejecuta en complejidad algorítmica lineal O (N), donde N es el número de enlaces en la cadena if-instanceof-else (es decir, el número de clases diferentes que se manejarán). Por lo tanto, el rendimiento de la solución basada en HashMap es asintóticamente más alto N veces que el rendimiento de la solución de cadena if-instanceof-else. Considere que necesita manejar diferentes descendientes de la clase de Mensaje de manera diferente: Mensaje1, Mensaje2, etc. A continuación se muestra el fragmento de código para el manejo basado en HashMap.

public class YourClass { 
    private class Handler { 
     public void go(Message message) { 
      // the default implementation just notifies that it doesn't handle the message 
      System.out.println(
       "Possibly due to a typo, empty handler is set to handle message of type %s : %s", 
       message.getClass().toString(), message.toString()); 
     } 
    } 
    private Map<Class<? extends Message>, Handler> messageHandling = 
     new HashMap<Class<? extends Message>, Handler>(); 

    // Constructor of your class is a place to initialize the message handling mechanism  
    public YourClass() { 
     messageHandling.put(Message1.class, new Handler() { public void go(Message message) { 
      //TODO: IMPLEMENT HERE SOMETHING APPROPRIATE FOR Message1 
     } }); 
     messageHandling.put(Message2.class, new Handler() { public void go(Message message) { 
      //TODO: IMPLEMENT HERE SOMETHING APPROPRIATE FOR Message2 
     } }); 
     // etc. for Message3, etc. 
    } 

    // The method in which you receive a variable of base class Message, but you need to 
    // handle it in accordance to of what derived type that instance is 
    public handleMessage(Message message) { 
     Handler handler = messageHandling.get(message.getClass()); 
     if (handler == null) { 
      System.out.println(
       "Don't know how to handle message of type %s : %s", 
       message.getClass().toString(), message.toString()); 
     } else { 
      handler.go(message); 
     } 
    } 
} 

Más información sobre el uso de variables de tipo de clase en Java: http://docs.oracle.com/javase/tutorial/reflect/class/classNew.html

0

He resuelto este problema utilizando reflection (alrededor de 15 años atrás en la era pre Genéricos).

GenericClass object = (GenericClass) Class.forName(specificClassName).newInstance(); 

He definido una clase genérica (clase base abstracta). He definido muchas implementaciones concretas de la clase base. Cada clase concreta se cargará con className como parámetro. Este nombre de clase se define como parte de la configuración.

La clase base define el estado común en todas las clases concretas y las clases concretas modificarán el estado anulando las reglas abstractas definidas en la clase base.

En ese momento, no sé el nombre de este mecanismo, que se ha conocido como reflection.

Pocas alternativas más se enumeran en este article: Map y enum aparte de la reflexión.

+0

Simplemente curioso, ¿por qué no hiciste 'GenericClass' una' interfaz'? – Ztyx

+0

Tuve un estado común y comportamiento predeterminado, que debe compartirse entre muchos objetos relacionados –

Cuestiones relacionadas