28 votos

¿Es este el diseño mal considerado operador "instanceof"?

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

  • Cambió 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.

27voto

Tomasz Nurkiewicz Puntos 140462

El Visitante 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 usted tiene que aplicar la lógica en todas partes, como no va a compilar lo contrario. Con instanceof de 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 regresar null cuando ninguno de los dos tipos encontrado.

3voto

ivowiblo Puntos 5455

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 debe ejecutar es junto a algo que el modelo no sabe (como interfaz de usuario) deberá utilizar un tipo de patrón doble de expedición o de visitante.

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

0voto

user949300 Puntos 7954

Otro método posible sería hacer process() (o tal vez llamarlo "doSubclassProcess()" si aclara cosas) abstracto (en TipoRegistro), y 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;
   }
}

Ten cuidado con un par de errores tipográficos - creo que los fija.

0voto

meriton Puntos 30447

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 de visitante patrón.

PS: En una revisión de código, me gustaría bandera return null, debido a que 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.

0voto

Lukasz Puntos 9471

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

Otro es el de 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 usar Map para el mapeo entre el apoyo class y Visitantes de la instancia. Gran if ... else bloque con instanceof cheques sería muy ineficaz.

Iteramos.com

Iteramos es una comunidad de desarrolladores que busca expandir el conocimiento de la programación mas allá del inglés.
Tenemos una gran cantidad de contenido, y también puedes hacer tus propias preguntas o resolver las de los demás.

Powered by:

X