foo-> bar-> foobarが悪いスタイルと見なされるのはなぜですか？コードを追加しないで避ける方法

Question

私たちのC++教授は、演算子の結果を>別の演算子への入力として使うことは、悪いスタイルとみなされると述べています。だからではなく、書き込みの

：

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

彼が好む：

Node* terminal = edge->terminal; 
return terminal->outgoing_edges[0]; 

1. これは悪いスタイルだと考えているのはなぜ？
2. 「悪いスタイル」を避けるためにプログラムを再構築することはできますが、上記の提案に従って作成された余分なコード行は避けてください。

Answer 1

教授になぜ彼はそれが悪いスタイルであると考えているのかを質問するべきです。私はしません。私はしかし、ターミナルの宣言で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. Maintanability
2. 効率

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

Answer 2

多くの理由があります。

Law of Demeterは構造的な理由があります（ただし、あなたのC++教授のコードはこれにも違反しています）。あなたの例では、edgeは約terminalとoutgoing_edgesを知っている必要があります。それは密接に結びついている。代替

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

として

今、あなたはどこにでも変更することなく、一つの場所にoutgoing_edgesの実装を変更することができます。正直言って、私は実際にグラフのようなデータ構造の場合はこれを買っていません（それは密接に結合されています、エッジとノードはお互いにエスケープできません）。これは私の本では過度に抽象化されるでしょう。

a->b->cの式でnull参照解除問題もありますが、bがnullの場合はどうなりますか？

Answer 3

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

2番目のコードスニペットよりもわずかに良いは次のようになります。私はoutgoing_edgesが配列であると仮定しています

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

。そうでない場合は、NULLか空であるかどうかをチェックする必要があります。

Answer 4

あなたの教授が何を意味していたのか分かりません。私はいくつかの考えを持っています。

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

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

あなたは第二の例は短いが、読みやすいだけでなく、どのように見ることができます。

あなたは、彼らが戻る何を気にする必要はありませんので、時々機能のチェーンは、簡単です。もう一つは、デバッグしている

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

対

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

。関数呼び出しの長いチェーンをデバッグするのは非常に難しいです。なぜなら、エラーの原因となっているものがどれだけあるかを理解できないからです。この場合、チェーンを破壊することは、それが良い」「悪い」かであると言うことはできません、あなたは要約するので

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

、のように、あまりにもいくつかの余分なチェックを行うことができますもちろんのこと、多くのことができます"一般的なスタイル。まずあなたの状況を考慮する必要があります。

Answer 5

どちらも現在の形式では必ずしも優れていません。しかし、2番目の例でnullのチェックを追加する場合、これは意味をなさないでしょう。

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

誰がoutgoing_edgesが正しく読み込ままたは割り当てられているということであるので、でもそれはまだ悪いですが。より良い賭けは、outgoingEdges()と呼ばれる関数を呼び出すことです。それはあなたにとって素晴らしいエラーチェックを行い、決してあなたを引き起こさないでしょう未定義の振舞い。