この "instanceof"オペレータの使用は悪い設計を考慮していますか？

Question

私のプロジェクトの1つでは、レコードタイプの抽象クラスを継承する2つの「データ転送オブジェクト」RecordType1とRecordType2があります。

両方のRecordTypeオブジェクトを、同じRecordProcessorクラスで "process"メソッド内で処理する必要があります。

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 
} 

私はスコット・マイヤーズは、以下で効果的なC++を書き込む読んだ：私の最初に考えたのは、2つの特定のプロセスメソッドに委譲を次のように汎用的な処理方法を作成することでした

「いつでもあなたオブジェクトのタイプがT1の場合はフォームのコードを記述してから、何かを実行しますが、タイプがT2の場合は他の操作を行います。

もし彼が正しければ、私は自分自身を叩いているはずです。私は実際にこれが悪いデザインであることは実際には分かりません（もちろん、誰かがRecordTypeをサブクラス化し、それを扱うジェネリックの "Process"メソッドに別の行を追加せずにRecordType3を追加しないとNPEを作成しません）理論的にはこれらのレコードで実行したい多くの異なるタイプの処理が可能なので、実際には私にはあまり意味がありません。

これは悪い設計とみなされる理由を説明し、これらのレコードを処理中のクラスに処理する責任を引き続き与える何らかの代替手段を提供できますか？

UPDATE：だけ明確にするthrow new IllegalArgumentException(record);

へreturn nullを変更

• 、シンプルRecordType.process（）メソッドは十分ではない三つの理由があります：まず、処理は本当にあまりにも遠くに除去されますRecordTypeのサブクラスで独自のメソッドを使用できるようにするためです。また、理論的には異なるプロセッサによって実行されることができる、異なるタイプの処理の完全なスルーが存在する。最後に、RecordTypeは、内部で定義された最小限の状態変更メソッドを持つ単純なDTOクラスとして設計されています。

Answer 1

典型的には、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たときにどちらを返しますタイプが見つかりました。

Answer 2

私の提案：

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

Answer 3

別の可能なアプローチは）プロセスを（作る（または、おそらくそれ「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; 
    } 
} 

いくつかのタイプミスがあるのを見てみてください。

Answer 4

デザインは目的や制約を知らずに終了する手段であり、誰かがあなたのデザインがその特定の状況に適しているかどうか、またどのように改善するかを判断することはできません。

しかし、オブジェクト指向設計では、メソッドの実装を別のクラスに保ち、各タイプごとに別々の実装を依然として持つ標準的なアプローチはvisitor patternです。

PS：コードレビューでは、報告するよりもむしろバグを伝播する可能性があるため、return nullにフラグを付けます。考えてみましょう：到達不能なコードパスは、未定義の動作を引き起こすのではなく例外をスローするはずです。

Answer 5

あなたの例のように1つのデザインが悪いと思われる場合は、訪問者パターン（該当する場合）を使用しないでください。

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

ビジターパターン、通常効果的でエレガントな解決策を使用する場合、支持classとビジターインスタンス間のマッピングのためMapを使用しています。大きいif ... elseブロックでinstanceofのチェックは非常に効果がありません。