が上がるようになるでしょうコーディングスタイル?
-
08-07-2019 - |
質問
中のコードレビュー、シニアdev参入れ子になったか自分のコードです。することが提案された設定しまbool値ように思いを一つ以上のレベルの営巣.こういったコードを読みやすくする工夫が取得したい♪その他の意見もある。が上がるようになるでしょう。は膝-ジャーク回避姿勢営巣に設立されたのですか?
以下の簡単なコード例です。
ネスト:
If(condition1)
{
If(condition2)
{
if(condition3)
{
return true;
}
else
{
log("condition3 failed");
}
else
{
log("condition2 failed")
}
}
else
{
log("condition1 failed")
}
return false;
または
Bool動:
bool bRC = false;
bRC = (condition1);
if(brc)
{
bRC = (condition2);
}
else
{
log("condition1 failed");
return false;
}
if(bRC)
{
bRC = (condition3);
}
else
{
log("condition2 failed");
return false;
}
if(bRC)
{
return true;
}
else
{
log("condition3 failed");
return false;
}
解決
私はあなたの方が好きですが、おそらく次のようなことをするでしょう:
if (condition1 && condition2 && condition3)
{
return true;
}
else if (!condition1)
{
log("condition1 failed");
}
else if (!condition2)
{
log("condition2 failed");
}
else
{
log("condition3 failed");
}
return false;
条件が複雑な式である場合、if文を評価する前に適切な名前の変数に式を割り当てて、各ifの値を再計算する必要を回避します。
これは、通常モードではすべての条件が真であるため、最初にそのチェックが必要であることを前提としています。通常モードで1つ以上の条件がfalseである場合、順序を変更して各否定を順番にチェックし、すべてのチェックが失敗した場合はtrueを返します。また、複雑な式の代わりに一時変数を使用する必要がなくなります。
他のヒント
複数のリターンポイントに関するばかげたルールがない場合、これは非常に良いことだと思います(他の人も同様ですが、不明な理由で回答を削除しました):
if(!condition1)
{
log("condition1 failed");
return false;
}
if(!condition2)
{
log("condition2 failed");
return false;
}
if(!condition3)
{
log("condition3 failed");
return false;
}
return true;
これは、スーパーネストに対するひざまずき嫌悪感と同じかもしれませんが、特定の値にブール条件を保存するよりも確かにきれいです。ただし、コンテキストでは読みにくい場合があります。条件の1つが isreadable()
であったかどうかを検討してください。何かが読めるかどうかを知りたいので、 if(isreadable())
と言う方が明確です。 if(!isreadable())
は、読み取り可能でないかどうかを知りたいかどうかを提案しますが、これは意図ではありません。一方が他方より読みやすく/直感的である状況があるかもしれないことは確かに議論の余地がありますが、私はこの方法のファンです。誰かが返品にハングアップした場合、これを行うことができます:
if(!condition1)
log("condition1 failed");
else if(!condition2)
log("condition2 failed");
else if(!condition3)
log("condition3 failed");
else
return true;
return false;
しかし、それはかなり下手であり、「明確」ではありません。私の意見では。
私は個人的に、ネストされたコードが非常に読みやすいと感じています。
私は通常、自分の条件にネストを好みます。もちろん、ネストされた条件が右側にインデントされすぎている場合、私がやろうとしていること(リファクタリング、再設計など)に取り組むより良い方法があるかどうか疑問に思う必要があります。
ネストされたバージョンに似ていますが、私にとってはずっときれいです:
if not condition1:
log("condition 1 failed")
else if not condition2:
log("condition 2 failed")
else if not condition3:
log("condition 3 failed")
else
return true;
return false;
各条件が1回評価されることに注意してください。
2番目のスタイルはばかげて冗長です。彼は本当にこれを正確に提案しましたか? else部分に return
があるため、これらの if
ステートメントのほとんどは必要ありません。
ネストされた例では、可能な else
句を含めることを忘れないでください。
これらのどちらも私には満足できないようです。
ブール駆動型の方法はわかりにくいです。必要に応じてネストは問題ありませんが、条件を1つのステートメントに結合するか、さらに評価が行われるメソッドを呼び出すことで、ネストの深さの一部を削除できます。
と思う方ができるとともに、一長一短がある。混雑して待たされることはbool型の風る場合においても深くネスト(8または10のようなものです。このケースでは3つのレベルは、選択スタイルの正確なサンプルから、今のように:
void YourMethod(...) { if (condition1 && condition2 && consition3) return true; if (!condition 1) log("condition 1 failed"); if (!condition 2) log("condition 2 failed"); if (!condition 3) log("condition 3 failed"); return result; }
...やのような希望の場合は単一の出口(い)...
void YourMethod(...) { bool result = false; if (condition1 && condition2 && consition3) { result = true; } else { if (!condition 1) log("condition 1 failed"); if (!condition 2) log("condition 2 failed"); if (!condition 3) log("condition 3 failed"); } return result; }
この方法について一覧を取得しました状態でログインしたのです。おとえば、一つだけに失敗した状態でログインが含まれていてもよ失敗します。
おそらく一緒に行きます
if (!condition1) log("condition 1 failed");
else if (!condition2) log("condition 2 failed");
else if (!condition3) log("condition 3 failed");
// -------------------------------------------
return condition1 && condition2 && condition3;
これは、同等で、はるかにクリーンだと思います...
また、最初に失敗したものだけでなく、失敗した場合にすべての条件を評価してログに記録することをクライアントが決定すると、これを行うための修正がはるかに簡単になります:
if (!condition1) log("condition 1 failed");
if (!condition2) log("condition 2 failed");
if (!condition3) log("condition 3 failed");
// -------------------------------------------
return condition1 && condition2 && condition3;
これは、実装が実際に目的の動作を反映している場合に、実装する方法です。
if (!condition1) {
log("condition1 failed");
return false;
}
if (!condition2) {
log("condition2 failed");
return false;
}
if (!condition3) {
log("condition3 failed");
return false;
}
return true;
ただし、失敗したすべての状態をログに記録する必要がある可能性が高いと思います。
result = true;
if (!condition1) {
log("condition1 failed");
result = false;
}
if (!condition2) {
log("condition2 failed");
result = false;
}
if (!condition3) {
log("condition3 failed");
result = false;
}
return result;
言語が例外処理をサポートしている場合、私は次のようにします:
try {
if (!condition1) {
throw "condition1 failed";
}
if (!condition2) {
throw "condition2 failed";
}
if (!condition3) {
throw "condition3 failed";
}
return true;
} catch (e) {
log(e);
return false;
}
チャールズ・ブレタナから編集:制御フローの例外の使用
どちらの方法も好きではありません。巣がたくさんあると、何かがおかしい。フォームの検証や、このような何かを実際に必要とするものの場合、よりモジュール式またはコンパクトなものを見つけようとします。
例としては、条件を保持する配列があります。この配列を介してしばらく繰り返し、必要に応じて印刷/中断します。
必要に応じて実装が多すぎるため、サンプルコードを作成しても意味がありません。
大雑把に言って、コードが複雑すぎるように見えるとき、それはひどいです:)。再考してみてください。ほとんどの場合、コーディング慣行に従うことで、コードはより美観的で短くなります。そして明らかに、よりスマートです。
コードは、指定された言語で問題を修正します。したがって、どちらのスニペットも「より良い」ものにできると考えています。モデル化される問題に依存します。私の推測では、どちらの解決策も実際の問題に対応するものではありません。 condition1,2,3の代わりに実際の条件を入力すると、「最高」が完全に変わる可能性があります。コード。
それをすべて一緒に書くより良い(3d)方法があると思う。
if( condition1 && condition2 && condition3 )
return true;
log(String.Format("{0} failed", !condition1 ? "condition1" : (!condition2 ? "condition2" : "condition3")));
return false;
その方法では、ロギングのためだけに多くのコード行を見る必要はありません。そして、すべての条件が当てはまる場合、ログに記録する必要がある場合にそれらを評価する時間を無駄にしないでください。