「ある」か「ある」か - コードの匂いで判断

StackOverflow https://stackoverflow.com/questions/733011

  •  06-09-2019
  •  | 
  •  

質問

昨日、Bar から継承する Foo クラスでこれを書きました。

public override void AddItem(double a, int b)
{
    //Code smell?
    throw new NotImplementedException("This method not usable for Foo items");
}

その後、これは私がそうすべきである可能性を示唆しているのではないかと疑問に思いました を使用して Bar から継承するのではなく、Bar を作成します。

継承と合成のどちらかを選択するのに役立つ「コードの匂い」は他にありますか?

編集 これは断片であり、他の方法もあることを付け加えておきます。 は 共通して言えるのは、あまり詳細には触れたくなかったということです。私はコンポジションに切り替えることの影響を分析する必要があり、バランスを崩すのに役立つ可能性のある他の「コードの匂い」があるのではないかと考えました。

役に立ちましたか？

解決

上で挙げた例は明らかにコードの臭いです。の AddItem メソッドは基本クラスの動作です Bar. 。もし Foo はサポートしません AddItem の動作を継承すべきではありません Bar.

より現実的な (C++) 例を考えてみましょう。次のクラスがあるとします。

class Animal
{
    void Breathe() const=0;
}

class Dog : public Animal
{
    // Code smell
    void Breathe() { throw new NotSupportedException(); }
}

基本抽象クラス Animal 純粋な仮想を提供します Breathe() なぜなら、動物は生きていくために呼吸をしなければならないからです。呼吸をしなければ、定義上、それは動物ではありません。

新しいクラスを作成することで Dog から継承するもの Animal しかし しません をサポートする Breathe() その行為は、規約で定められた契約に違反することになります。 Animal クラス。かわいそうな犬は生き残れないでしょう！

パブリック継承の簡単なルールは、派生クラス オブジェクトが本当に基本クラス オブジェクトである場合にのみパブリック継承を行う必要があるということです。

あなたの特定の例では:

  • Foo はサポートしません AddItem() に定められた行為 Bar 契約。
  • したがって、定義により、 Foo 「ではない」 Bar, 、そしてそれを継承すべきではありません。

他のヒント

fooは拡張せずにバーのように動作しない場合は、

なぜあなたは、バーから継承するのでしょうか？ 私も、基本クラスで仮想として「AddItemメソッド」のようなメソッドを宣言しないと、考えてみれば。

  

いいえ！正方形は、長方形であるかもしれないが、   スクエアオブジェクトは間違いではありません   矩形オブジェクト。どうして？なぜなら   スクエアオブジェクトの振る舞いではありません   の動作と一貫性   矩形オブジェクト。 を行動的に、   広場は長方形ではありません！そして、それは、   ソフトウェアは本当にすべてであることを行動   程度。の

のオブジェクト内からメンターます。

はい、あなたはメソッド「未実施」しなければならないことは、おそらくあなたは、「ある-」の関係を使うべきではないことを示しています。あなたのFOOSは本当にバーしていないようです。

しかし、あなたのFOOSとバーを考えることから始めます。 FOOSバーはありますか？あなたはセットとサブセットは、紙の上に描くことができ、すべてのFooのは（つまり、Fooのクラスのすべてのmemeberである）もバー（つまり、バークラスのメンバーである）でしょうか？ない場合は、おそらく継承を使用する必要はありません。

FOOSは本当にバーではないことを示している、とfooはバーを継承してはならないことを別のコードのにおいは、あなたがポリモーフィズムを使用することができないということです。あなたが引数としてバーがかかりますが、Fooのを処理することはできません方法を考えてみましょう。 （おそらくそれはその引数でAddItemメソッドを呼び出しますので！）あなたは、コードが複雑で理解しにくい作り、チェックを追加したり、NotImplementedExceptionを処理する必要があります。 （そして臭い！）

あなたのFooはちょうどそれが所有するバーにメッセージを転送するインタフェースの多くを実装している場合、直接の会話があります。もちろん、これはがなければならないことのバーを示している可能性があり。のみ、しかし、臭いは常に正しいではありませんでした。

おそらく、上記の例では、何かがうまくいかないことを示していますが、

、NotImplementedException自体は常に間違っていないです。これは、すべてのスーパークラスの契約については、この契約を実装するサブクラスについてです。あなたのスーパークラスは、このような方法を持っている場合、

// This method is optional and may be not supported
// If not supported it should throw NotImplementedException 
// To find out if it is supported or not, use isAddItemSupported()
public void AddItem(double a, int b){...}

すると、このメソッドをサポートしていないことは、まだ契約してokです。それがサポートされていない場合は、おそらくUIに対応するアクションを無効にする必要があります。あなたは、このような契約で大丈夫ですのであれば、そのサブクラスは、それを正しく実装しています。

クライアントが明示的にそれはすべてのクラスのメソッドを使用していないとするつもりはありませんことを宣言

別のオプション。このような

// This method never modifies orders, it just iterates over 
// them and gets elements by index.
// We decided to be not very bureaucratic and not define ReadonlyList interface,
// and this is closed-source 3rd party library so you can not modify it yourself.
// ha-ha-ha
public void saveOrders(List<Order> orders){...}

それから、追加、削除、および他のミューテータサポートしていないリストのそこimplementaionを渡すためにokです。ちょうどそれを文書化します。

// Use with care - this implementation does not implement entire List contract
// It does not support methods that modify the content of the list.
public ReadonlyListImpl implements List{...}
すべてのご契約を定義するためにあなたのコードを持つことは良いですが、それはあなたが契約に違反しているかどうかを確認するためのコンパイラを聞かせて

、、時にはそれが合理的ではないですし、コメントのような弱い定義されたために契約を、頼る必要があります。

あなたが本当に安全コードだけではありませんスーパークラスは、その契約によって定義されていることを考慮して、スーパーなどのサブクラスを使用することができれば、それは、質問に来る短い言葉でます。

.NET Frameworkが特にSystem.IO名前空間には、このような例があります - 一部の読者は、その基本クラスのプロパティ/メソッドをすべて実装していない、あなたはそれらを使用しようとした場合、例外がスローされます。

。

例えば。ストリームは、positionプロパティを持っていますが、一部のストリームがこれをサポートしていません。

組成物は、複雑さを軽減した継承する好ましいにあります。より良いアプローチは、コンストラクタ・インジェクションを使用して、あなたのFooクラスのバーへの参照を維持するだろうます。

ライセンス： CC-BY-SA と 帰属
所属していません StackOverflow
scroll top