Es Este Uso de la "instanceof" Operador Considera de Mal Diseño?

Question

En uno de mis proyectos, tengo dos "objetos de transferencia de datos" RecordType1 y RecordType2 que hereda de una clase abstracta de RecordType.

Quiero tanto RecordType objetos para ser procesados por el mismo RecordProcessor clase dentro de un "proceso" método.Mi primer pensamiento fue para crear un proceso genérico método que los delegados a los dos métodos de proceso de la siguiente manera:

public RecordType process(RecordType record){

    if (record instanceof RecordType1)
        return process((RecordType1) record);
    else if (record instanceof RecordType2)
        return process((RecordType2) record);

    throw new IllegalArgumentException(record);
}

public RecordType1 process(RecordType1 record){
    // Specific processing for Record Type 1
}

public RecordType2 process(RecordType2 record){
    // Specific processing for Record Type 2
}

He leído que Scott Meyers, escribe lo siguiente en Efectivos De C++ :

"En cualquier momento usted se encuentra en la escritura del código de la forma 'si el objeto es de tipo T1, a continuación, hacer algo, pero si es de tipo T2, y luego hacer algo más,' bofetada a ti mismo."

Si él está en lo correcto, es evidente que debe ser abofetear a mí mismo.Yo no veo cómo esto es un mal diseño (a menos, claro, que alguien subclases RecordType y agrega en un RecordType3 sin necesidad de añadir otra línea genérico "Proceso", método que se encarga, por lo tanto la creación de un NPE), y las alternativas no puedo pensar en involucrar a poner el peso de ese tratamiento específico dentro de la lógica de la RecordType clases de sí mismos, que realmente no tiene mucho sentido para mí, ya no puede ser, en teoría muchos tipos diferentes de procesamiento que me gustaría realizar en estos registros.

Puede alguien explicar por qué esto podría ser considerado un mal diseño y proporcionar algún tipo de alternativa que todavía le da la responsabilidad de la tramitación de estos expedientes a un "Tratamiento" de la clase?

ACTUALIZACIÓN:

• Cambiado return null a throw new IllegalArgumentException(record);
• Solo para aclarar, hay tres razones por las que un simple RecordType.método process() no sería suficiente:En primer lugar, el procesamiento de la realidad está muy lejos de RecordType para merecer su propio método en el RecordType subclases.También, hay un montón de diferentes tipos de tratamiento que, en teoría, podría ser realizado por los diferentes procesadores.Finalmente, RecordType está diseñado para ser una simple clase DTO con un estado mínimo-el cambio en los métodos definidos dentro.

Answer 1

El Visitante el patrón se suele utilizar en estos casos.Aunque el código es un poco más complicado, pero después de la adición de un nuevo RecordType subclase que tiene que aplicar la lógica en todas partes, como no va a compilar lo contrario.Con instanceof todo el lugar es muy fácil perder uno o dos lugares.

Ejemplo:

public abstract class RecordType {
    public abstract <T> T accept(RecordTypeVisitor<T> visitor);
}

public interface RecordTypeVisitor<T> {
    T visitOne(RecordType1 recordType);
    T visitTwo(RecordType2 recordType);
}

public class RecordType1 extends RecordType {
    public <T> T accept(RecordTypeVisitor<T> visitor) {
        return visitor.visitOne(this);
    }
}

public class RecordType2 extends RecordType {
    public <T> T accept(RecordTypeVisitor<T> visitor) {
        return visitor.visitTwo(this);
    }
}

De uso (nota genérico tipo de retorno):

String result = record.accept(new RecordTypeVisitor<String>() {

    String visitOne(RecordType1 recordType) {
        //processing of RecordType1
        return "Jeden";
    }

    String visitTwo(RecordType2 recordType) {
        //processing of RecordType2
        return "Dwa";
    }

});

También me gustaría recomendar lanzar una excepción:

throw new IllegalArgumentException(record);

en lugar de devolver null cuando ninguno de los dos tipos encontrado.

Answer 2

Mi sugerencia:

public RecordType process(RecordType record){
    return record.process();
}

public class RecordType
{
    public RecordType process()
    {
        return null;
    }
}

public class RecordType1 extends RecordType
{
    @Override
    public RecordType process()
    {
        ...
    }
}

public class RecordType2 extends RecordType
{
    @Override
    public RecordType process()
    {
        ...
    }
}

Si el código que usted necesita para ejecutar está acoplado a algo que el modelo no debería saber (como la interfaz de usuario), entonces usted tendrá que utilizar una especie de doble de expedición o de visitante patrón.

http://en.wikipedia.org/wiki/Double_dispatch

Answer 3

Otro enfoque posible sería hacer un proceso de () de (o quizás lo llaman "doSubclassProcess()" si que aclara las cosas) resumen (en RecordType), y tienen las implementaciones reales en las subclases.por ejemplo,

class RecordType {
   protected abstract RecordType doSubclassProcess(RecordType rt);

   public process(RecordType rt) {
     // you can do any prelim or common processing here
     // ...

     // now do subclass specific stuff...
     return doSubclassProcess(rt);
   }
}

class RecordType1 extends RecordType {
   protected RecordType1 doSubclassProcess(RecordType RT) {
      // need a cast, but you are pretty sure it is safe here
      RecordType1 rt1 = (RecordType1) rt;
      // now do what you want to rt
      return rt1;
   }
}

Mira para un par de errores tipográficos - creo que he arreglado todos ellos.

Answer 4

El diseño es un medio para un fin, y no el conocimiento de su objetivo o las restricciones, que nadie le puede decir si su diseño es muy bueno en esa situación en particular, o cómo se puede mejorar.

Sin embargo, en el diseño orientado a objetos, el método estándar para mantener el método de implementación en una clase independiente sin dejar de tener una implementación independiente para cada tipo es la visitante patrón.

PS:En una revisión de código, me gustaría bandera return null, porque puede propagar errores en lugar de la presentación de informes.Considere la posibilidad de:

RecordType processed = process(new RecordType3());

// many hours later, in a different part of the program

processed.getX(); // "Why is this null sometimes??"

Dicho de otra manera, supuestamente inalcanzable rutas de código debe lanzar una excepción en lugar de resultar en un comportamiento indefinido.

Answer 5

Mal diseño en el que uno piensa, como en tu ejemplo, no utilizar visitante patrón, cuando sea aplicable.

Otro es la eficiencia. instanceof es bastante lento, en comparación con otras técnicas, tales como la comparación contra class objeto con la igualdad.

Cuando se utiliza visitante patrón, generalmente de una efectiva y elegante solución es el uso de Map para el mapeo entre el apoyo class y Visitante ejemplo.Grandes if ... else bloque con instanceof comprueba sería muy ineficaz.