16

私たちのC++教授は、operator->の結果を別のoperator->への入力として使用することは悪いスタイルと見なされると述べました。

したがって、書く代わりに：

return edge->terminal->outgoing_edges[0];

彼は好むだろう：

Node* terminal = edge->terminal;
return terminal->outgoing_edges[0];
  1. なぜこれが悪いスタイルと見なされるのですか？
  2. 「悪いスタイル」を回避するだけでなく、上記の提案に従って作成される余分なコード行を回避するために、プログラムを再構築するにはどうすればよいですか？
4

5 に答える 5

19

いくつかの理由があります。

デメテルの法則は構造的な理由を示しています（ただし、C ++の教授のコードはまだこれに違反していることに注意してください！）。あなたの例でedgeは、とについて知っている必要がterminalありoutgoing_edgesます。それはそれを緊密に結合させます。

別の方法として

class Edge {
 private:
   Node* terminal;
 public:
   Edges* outgoing_edges() {
      return terminal->outgoing_edges;
   }
}

これで、どこでも変更せずに、の実装をoutgoing_edges1か所で変更できます。正直なところ、グラフのようなデータ構造の場合、これを実際に購入することはありません（緊密に結合されており、エッジとノードが互いにエスケープできません）。これは私の本では過度に抽象化されているでしょう。

null逆参照の問題もあります。式a->b->cには、nullの場合bはどうなりますか？

于 2012-07-10T13:11:43.157 に答える
12

なぜ彼がそれを悪いスタイルだと考えるのかについて教授に尋ねるべきです。私はしません。しかし、私は彼のターミナル宣言でのconstの省略は悪いスタイルだと思います。

そのような単一のスニペットの場合、それはおそらく悪いスタイルではありません。ただし、これを考慮してください。

void somefunc(Edge *edge)
{
   if (edge->terminal->outgoing_edges.size() > 5)
   {
        edge->terminal->outgoing_edges.rezize(10);
        edge->terminal->counter = 5;
   }
   ++edge->terminal->another_value;
}

これは扱いにくくなり始めています-読むのも書くのも難しいです（私はそれをタイプするときに約10のタイプミスをしました）。そしてそれはそれらの2つのクラスで演算子->の多くの評価を必要とします。オペレーターが些細なことであればOKですが、オペレーターが何かエキサイティングなことをすると、多くの作業を行うことになります。

したがって、2つの可能な答えがあります。

  1. メンテナンス性
  2. 効率

そして、そのような単一のスニペットでは、余分な行を避けることはできません。上記のようなものでは、タイピングが少なくなります。

于 2012-07-10T13:15:08.207 に答える
6

ええと、あなたの教授が何を意味したのかはわかりませんが、私にはいくつかの考えがあります。

まず第一に、このようなコードは「それほど明白ではない」可能性があります。

someObjectPointer->foo()->bar()->foobar()

の結果が何であるfoo()かを実際に言うことはできません。したがって、何がbar()呼ばれているのかを実際に言うことはできません。それは同じオブジェクトですか？それとも、作成中の一時オブジェクトですか？または多分それは何か他のものですか？

もう1つは、コードを繰り返す必要がある場合です。次のような例を考えてみましょう。

complexObject.somePart.someSubObject.sections[i].doSomething();
complexObject.somePart.someSubObject.sections[i].doSomethingElse();
complexObject.somePart.someSubObject.sections[i].doSomethingAgain();
complexObject.somePart.someSubObject.sections[i].andAgain();

そのようなものと比較してください：

section * sectionPtr = complexObject.somePart.someSubObject.sections[i];
sectionPtr->doSomething();
sectionPtr->doSomethingElse();
sectionPtr->doSomethingAgain();
sectionPtr->andAgain();

2番目の例が短いだけでなく、読みやすくなっていることがわかります。

関数が返すものを気にする必要がないため、関数のチェーンが簡単な場合があります。

resultClass res = foo()->bar()->foobar()

vs

classA * a = foo();
classB * b = a->bar();
classC * c = b->foobar();

もう1つはデバッグです。関数呼び出しの長いチェーンをデバッグすることは非常に困難です。なぜなら、それらのどれがエラーの原因であるかを単純に理解できないからです。この場合、チェーンを壊すことは大いに役立ちます。言うまでもなく、次のような追加のチェックも行うことができます。

classA * a = foo();
classB * b;
classC * c;
if(a)
   b = a->bar();
if(b)
   c = b->foobar();

したがって、要約すると、一般的に「悪い」または「良い」スタイルであるとは言えません。最初に自分の状況を考慮する必要があります。

于 2012-07-10T13:17:16.450 に答える
3

表示した2つのコードスニペットは（もちろん）意味的に同一です。スタイルの問題に関しては、教授が望んでいることに従うことをお勧めします。

2番目のコードスニペットよりも少し優れています：

Node* terminal = (edge ? edge->terminal : NULL);
return (terminal ? terminal->outgoing_edges[0] : NULL);

私はoutgoing_edgesが配列であると仮定しています。そうでない場合は、それがNULLまたは空であることも確認する必要があります。

于 2012-07-10T13:16:13.177 に答える
1

どちらも、現在の形で必ずしも優れているわけではありません。ただし、2番目の例でnullのチェックを追加する場合、これは理にかなっています。

if (edge != NULL)
{
   Node* terminal = edge->terminal;
   if (terminal != NULL) return terminal->outgoing_edges[0];
}

それでも、それoutgoing_edgesが適切に設定または割り当てられていると誰が言うのかという理由で、それでも悪いことです。より良い方法は、呼び出された関数を呼び出すことです。この関数は、エラーチェックをすべて実行し、未定義の動作outgoingEdges()を引き起こすことはありません。

于 2012-07-10T13:41:52.210 に答える