2017-12-20 34 views
2

三項条件の結果としてメソッドを使用するのは悪い考えですか?線に沿って 何か:三元条件の結果のメソッド

(node->parent()->left() == node ? node->parent()->left() 
           : node->parent()->right()).reset(); 

は、私は、コードスタイルは主観的であり得ることを認識してんだけど、私はまだには、それを使用するか、むしろのようなものを書くかのように、私はいくつかの推薦を得ることができることを望む:

if (node–>parent()–>left() == node) { 
    node->parent()->left().reset() 
} else { 
    node–>parent()->right().reset() 
} 
何について(コメントより)

auto& childUnq = node->parent()->left() == node ? node->parent()->left() 
               : node->parent()->right(); 
childUnq.reset(); 
+1

なぜ最後の2つのオペランドが等しいのですか?最初に3進演算子は必要ありません。 – stackptr

+0

@stackptr私の悪いことに、私は誤って同じ場所に書きました。 – Davar

+0

条件式の結果を変数に格納することもできます。あなたは 'auto'で型を綴る必要さえありません。 – chris

答えて

2

私はそれは確かに悪いスタイルだと思います。

このように考えると、他の人のコードでそのコードを見つけたとしても、一見したことが分からない可能性があります。 reset()関数が最初に存在し、それが条件の両方の分岐に影響することに気づくには時間がかかるでしょう!

あなたにとっても、デバッグ中に不要な問題が発生する可能性があります。

3元演算子は単純なものに対してはきちんとしているかもしれませんが、それを酷使すると、コードはあなただけでなく他の人にとっても非常に読みにくくなります。 他の人のコードで見つかった場合は、これを見ないでください。

if(node->parent()->left() == node){ 
    node->parent_->left_.reset(); 
} else { 
    node->parent_->right_.reset(); 
} 

できるだけきれいにしてください。誰もがあなたに感謝します!私は強く、他の代替をお勧めし

3

:単純な名前を導入することにより

auto& left = node->parent()->left(); 
auto& right = node->parent()->right(); 
auto& pick = (left==node) ? left : right; 
pick.reset(); 

を、それは何が起こっているかは明らかです。三項演算子はそれ自体悪くないが、単純な文脈でのみ使用することが課題である。

+0

私はこのソリューションが本当に好きですが、私の一部はパフォーマンスを心配していますが、それはおそらく無駄なマイクロ最適化でしょうか?私の情報のために、このような新しい参照変数を作成するとどれくらいの影響がありますか? – Davar

+0

オプティマイザは、 'Node :: left'に副作用があるかどうかを気にせず、そうでなければ全く同じ性能を発揮します。 – Caleth

+0

@Davar:これは単に名前を割り当て、名前はパフォーマンスに影響しません。 – MSalters

関連する問題