2017-05-28 10 views
0

私は自分のアプリケーションにとって重要である単純なコピー/クローンの方法があります:コピーしてはならないいくつかの特定のフィールドがあることを新しいフィールドが追加されたときにクローン/コピーの方法を破らないようにするにはどうすればよいですか?

@Override 
    public Operation getCopy() { 
    Operation copy = new Operation(); 
    copy.year = this.year; 
    copy.stage = this.stage; 
    copy.info = this.info; 
    copy.user = this.user.getCopy(); 
    // NOT TO BE COPIED! copy.id = this.id; 
    ... 
    return copy; 
} 

注意を。また、独自のコピー方法を持つ複雑なオブジェクト(Userなど)もあります。

問題は時々、開発者がコピーされるべき新しいフィールドを作成しますが、彼はcopyメソッドに追加するのを忘れ、新しいコードとして開発されていることである。

private String additionalInfo; 

そして、そこではないにもかかわらず、コンパイルエラーが発生した場合、私たちのQAチームによって、またはユーザーによってさえ後で発見されるビジネス上の問題があります。

これを防ぐにはどうすればよいですか?私は元のオブジェクトとそのコピーを比較するJUnitテストを試しましたが、既存のフィールドではうまく動作しますが、新しいフィールドを考慮していません。

答えて

2

は、私は、このための「ループとスイッチ」テスト呼ん使用:

for (Field field : Operation.class.getFields()) { 
    switch (field.getName()) { 
    case "year": 
     // Test that year is copied correctly. 
     // Initialize blah so that year is set. 
     assertEquals(getCopy(blah).year, blah.year); 
     break; 
    case "stage": 
     // Test that stage is copied correctly. 
     // Initialize blah so that stage is set. 
     assertEquals(getCopy(blah).stage, blah.stage); 
     break; 
    case "id": 
     // We don't want to copy id. 
     // Initialize blah so that id is set. 
     assertNull(getCopy(blah).id); 
     break; 

    // etc. 

    default: 
     throw new AssertionError("Unhandled field: " + field.getName()); 
    } 
} 

それは非常に想像力豊か名前ではありません:クラスのフィールドのすべての上に、あなたのループ、その後、すぐに切り替えて、個々のフィールドを別々に明示的に処理することができます。

これの利点は、新たに追加されたフィールドの処理の欠如が、defaultのケースによって直ちにキャッチされることです。あなたは、テストでそれを処理する必要があると言って顔をすると大きな打ちのめす、さらにはプロダクションコードでもそれを処理する必要があることを拡張します。

プレーンな古いJavaリフレクションを使用する場合の欠点は、削除されるフィールドをキャッチしないことです。これは、生産コードにテストされていないコードパスがあるのではなく、未使用のコードなので、「悪くない」状況になる可能性があります。


私はプロトコルバッファ・プロトコルバッファ・コンバータを構築しながら、(私は残念ながら思い出すことができない、またはどこかで読ん)このイディオムを開発しました。あなたが実際に名前ではなく、フィールド番号に切り替えることができるようにするJavaプロトコルバッファは、generated field numbersを持っている:

for (FieldDescriptor fieldDesc : proto.getDescriptorForType().getFields()) { 
    switch (fieldDesc.getNumber()) { 
    case FIELD1_FIELD_NUMBER: 
     // ... 
    case FIELD2_FIELD_NUMBER: 
     // ... 
    } 
} 

フィールド番号はもはやので、これについての素晴らしい事は、あなたがあまりにも削除例を知ることですテストスイッチがコンパイルされなくなることを意味します。

+0

このアプローチは、変更があっても非常に不安定です。通常は 'assertEquals(copy、original、" copy failed ");'をテストすれば十分です。 –

+0

@LewBlochこれは、すべてのフィールドをチェックする 'equals'に頼っています。 –

+0

平等を確立するために必要なもののみ。状態が他のフィールドによって決定された場合、それらは 'equals'などで説明されるべきです。 –

0

どのようにしてコードレビューでオーバーライドが欠落していましたか?

また、コピー方法は脆くあってはいけません。 「コピーしないでください」という特定のフィールドがある場合、それらは子クラスに表示されるのはなぜですか?

なぜ継承階層が深いのですか?コピーが必要なすべてのタイプがCopyableインターフェイスを実装するのであれば、開発中にオーバーライドの欠如を逃すのはずっと難しく、深い継承階層は必要ありません。 getCopy()の適切な基本実装を継承したクラスは、継承されたインタフェースメソッドを実装するだけでなく、開始するためにsuper.経由で呼び出すことができます。

コンパイラを使用して、具体的な実装からメソッドをオーバーライドさせることはできません。コードレビューはそのような間違いを犯すことになっています。彼らはそれを逃した査読者と言葉を持っていない場合。

抽象メソッドの実装は、コンパイラが黙っているために捕捉するのが簡単です。

関連する問題