30

私のプロジェクトの 1 つに、RecordType の抽象クラスから継承する 2 つの「データ転送オブジェクト」RecordType1 と RecordType2 があります。

「プロセス」メソッド内で、両方の RecordType オブジェクトを同じ RecordProcessor クラスで処理したいと考えています。私が最初に考えたのは、次のように 2 つの特定のプロセス メソッドに委譲する汎用プロセス メソッドを作成することでした。

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
}

Scott Meyers が効果的な C++で次のように書いていることを読みました。

「『オブジェクトがタイプ T1 の場合は何かを実行し、タイプ T2 の場合は別のことを実行する』という形式のコードを書いていることに気付いたときはいつでも、自分を叩いてください。」

彼が正しければ、明らかに私は平手打ちをしているはずです。これがどのように悪い設計であるかは実際にはわかりません (もちろん、誰かが RecordType をサブクラス化し、RecordType3 を処理する一般的な「Process」メソッドに別の行を追加せずに RecordType3 を追加して、NPE を作成しない限り)、および私が考えることができる代替案特定の処理ロジックを RecordType クラス自体の中に置く必要がありますが、理論的にはこれらのレコードに対して実行したいさまざまな種類の処理が存在する可能性があるため、これはあまり意味がありません。

これが悪い設計と見なされる理由を誰かが説明し、これらのレコードを処理する責任を「処理」クラスに与える何らかの代替手段を提供できますか?

アップデート:

  • return nullに変更throw new IllegalArgumentException(record);
  • 簡単に説明すると、単純な RecordType.process() メソッドでは不十分な理由が 3 つあります。まず、処理が RecordType から離れすぎているため、RecordType サブクラスで独自のメソッドを作成する価値がありません。また、理論的にはさまざまなプロセッサで実行できるさまざまな種類の処理が多数あります。最後に、RecordType は、最小限の状態変更メソッドが定義された単純な DTO クラスになるように設計されています。
4

6 に答える 6

31

このような場合、通常、Visitorパターンが使用されます。コードはもう少し複雑ですが、新しいRecordTypeサブクラスを追加した後は、他の方法ではコンパイルされないため、どこにでもロジックを実装する必要があります。instanceofいたるところにあるので、1つか2つの場所を見逃すのは非常に簡単です。

例:

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

使用法(ジェネリックリターンタイプに注意):

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

});

また、例外をスローすることをお勧めします。

throw new IllegalArgumentException(record);

nullどちらのタイプも見つからない場合に戻る代わりに。

于 2012-01-12T20:11:38.217 に答える
3

私のおすすめ:

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

実行する必要があるコードが、モデルが認識すべきではないもの (UI など) と結合している場合は、一種のダブル ディスパッチまたはビジター パターンを使用する必要があります。

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

于 2012-01-12T20:16:31.763 に答える
0

デザインは目的を達成するための手段であり、目標や制約がわからない場合、その特定の状況でデザインが優れているかどうか、またはどのように改善されるかは誰にもわかりません。

ただし、オブジェクト指向設計では、メソッドの実装を別のクラスに保持しながら、タイプごとに個別の実装を維持するための標準的なアプローチは、ビジターパターンです。

return nullPS:コードレビューでは、バグを報告するのではなく伝播する可能性があるため、フラグを立てます。検討:

RecordType processed = process(new RecordType3());

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

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

言い換えると、到達不能と思われるコードパスは、未定義の動作を引き起こすのではなく、例外をスローする必要があります。

于 2012-01-12T20:26:08.550 に答える
0

別の可能なアプローチは、process() (または、それが明確になる場合は「doSubclassProcess()」と呼ぶ) を (RecordType で) 抽象化し、サブクラスに実際の実装を含めることです。例えば

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

いくつかのタイプミスに注意してください。すべて修正したと思います。

于 2012-01-12T20:23:17.753 に答える
0

あなたの例のように、該当する場合はビジターパターンを使用しないと、ある考えでは悪いデザインです。

もう 1 つは効率です。等式を使用してオブジェクトinstanceofと比較するなどの他の手法と比較して、非常に遅いです。class

ビジターパターンを使用する場合、通常、効果的で洗練されたソリューションは、サポートとビジターインスタンスMap間のマッピングに使用されます。チェック付きの大きなブロックは非常に効果的ではありません。classif ... elseinstanceof

于 2012-01-12T22:26:30.490 に答える
0

SOLIDの開閉原理に反する

于 2020-01-26T19:15:36.430 に答える