Wird diese Verwendung des „Instanz“ -Operators als schlechtes Design angesehen?

Question

In einem meiner Projekte habe ich zwei "Datenübertragungsobjekte" RecordType1 und RecordType2, die von einer abstrakten Klasse von RecordType erben.

Ich möchte, dass beide RecordType -Objekte innerhalb einer "Prozess" -Methode von derselben Datensatzprozessor -Klasse verarbeitet werden. Mein erster Gedanke war, eine generische Prozessmethode zu erstellen, die wie folgt an zwei spezifische Prozessmethoden delegiert:

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
}

Ich habe gelesen, dass Scott Meyers Folgendes schreibt Effektives C ++ :

"Jedes Mal, wenn Sie sich für den Code des Formulars schreiben, wenn das Objekt vom Typ T1 ist, dann tun Sie etwas, aber wenn es vom Typ T2 ist, dann tun Sie etwas anderes, schlägt sich selbst."

Wenn er Recht hat, sollte ich eindeutig mich schlagen. Ich sehe nicht wirklich, wie schlechtes Design ist (es sei denn natürlich jemand Unterklassen RecordType und fügt in einem RecordType3 hinzu, ohne der generischen "Prozess" -Methode, die es umgeht, eine weitere Zeile hinzuzufügen In den Datensatzklassen selbst ist die Hauptlast der spezifischen Verarbeitungslogik selbst in die Einstellung zu bringen, was für mich wirklich nicht viel Sinn macht, da es theoretisch viele verschiedene Arten der Verarbeitung geben kann, die ich in diesen Aufzeichnungen ausführen möchte.

Kann jemand erklären, warum dies als schlechtes Design angesehen werden könnte, und eine Alternative liefern, die immer noch die Verantwortung für die Verarbeitung dieser Aufzeichnungen für eine "Verarbeitungskurs" übernimmt?

AKTUALISIEREN:

• Geändert return null zu throw new IllegalArgumentException(record);
• Nur um zu verdeutlichen, gibt es drei Gründe, warum eine einfache Datensatztype.Process () -Methode nicht ausreichen würde: Erstens ist die Verarbeitung wirklich zu weit von RecordType entfernt, um eine eigene Methode in der RecordType -Unterklasse zu verdienen. Es gibt auch eine ganze Reihe verschiedener Verarbeitungsarten, die theoretisch von verschiedenen Prozessoren durchgeführt werden können. Schließlich ist RecordType als einfache DTO-Klasse mit minimalen staatlich ändernden Methoden ausgelegt, die in darin definierter Methoden sind.

Answer 1

Das Besucher Muster wird in solchen Fällen normalerweise verwendet. Obwohl der Code etwas komplizierter ist, aber nach dem Hinzufügen eines neuen RecordType Unterklasse Sie müssen, zu ... haben Implementieren Sie die Logik überall, da sie sonst nicht kompiliert wird. Mit instanceof Überall ist es sehr einfach, ein oder zwei Stellen zu verpassen.

Beispiel:

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

Verwendung (beachten Sie den generischen Rückgabetyp):

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

});

Ich würde auch empfehlen, eine Ausnahme zu werfen:

throw new IllegalArgumentException(record);

anstatt zurückzukehren null Wenn keiner der Typen gefunden wird.

Answer 2

Mein Vorschlag:

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()
    {
        ...
    }
}

Wenn der Code, den Sie ausführen müssen, an etwas gekoppelt ist, das das Modell nicht kennen sollte (wie die Benutzeroberfläche), müssen Sie eine Art Doppel -Versand- oder Besuchermuster verwenden.

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

Answer 3

Ein weiterer möglicher Ansatz wäre es, den Prozess () (oder vielleicht "dosubClassProcess ()" zu erstellen, wenn dies die Dinge verdeutlicht) abstrakt (in RecordType) und die tatsächlichen Implementierungen in den Unterklassen haben. z.B

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

Achten Sie auf ein paar Tippfehler - denken Sie, ich habe sie alle behoben.

Answer 4

Design ist ein Mittel zum Zweck, und wenn Sie nicht Ihr Ziel oder Ihre Einschränkungen kennen, kann niemand erkennen, ob Ihr Design in dieser bestimmten Situation gut ist oder wie es verbessert werden könnte.

In objektorientiertem Design ist der Standardansatz jedoch die Methode -Implemention in einer separaten Klasse und ist jedoch für jeden Typ eine separate Implementierung Besuchermuster.

PS: In einer Code -Bewertung würde ich markieren return null, weil es Fehler ausbreiten könnte, anstatt sie zu melden. In Betracht ziehen:

RecordType processed = process(new RecordType3());

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

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

Anders ausgedrückt, sollten angeblich nicht erreichbare Codepfade eine Ausnahme auswerfen, anstatt zu undefiniertem Verhalten zu führen.

Answer 5

Schlechtes Design in einem Denken, wie in Ihrem Beispiel, nicht verwendet Besucher Muster, gegebenenfalls zutreffend.

Ein anderer ist Effizienz. instanceof ist im Vergleich zu anderen Techniken ziemlich langsam, z. B. im Vergleich mit class Objekt mit Gleichheit.

Beim Benutzen Besucher Muster, normalerweise eine effektive und elegante Lösung, wird verwendet Map Zur Kartierung zwischen Unterstützung class und Besucher Beispiel. Groß if ... else Block mit instanceof Schecks wären sehr ineffektiv.