coding-style - 関数内の if ステートメントで最善の方法は何ですか

Question

大量の条件付き関数がある場合、それを整理する最良の方法は何ですか?

私が心配しているのは、他の誰かがコードに入ってきて、何が起こっているのかを理解していることです。例は単純ですが、条件が非常に複雑であることを想像してください。

例:

public void function(string value, string value2)
{
    if (value == null)
        return;

    if (value2 == value)
        DoSomething();
}

また

public void function(string value, string value2)
{
    if (value != null)
    {
        if (value2 == value)
            DoSomething();
    }
}

また

public void function(string value, string value2)
{
    if (value != null && value2 == value)
        DoSomething();
}

score 8 · Accepted Answer

条件を整理し、メソッドに入れます。

たとえば、これを置き換えます。

 if( a& & n || c  && ( ! d || e ) && f > 1 && ! e < xyz ) { 
      // good! planets are aligned.
      buyLotteryTicket();
 } else if( ..... oh my ... ) { 
 }

これに：

if( arePlanetsAligned() ) { 
    buyLotteryTicket(); 
} else if( otherMethodHere() ) { 
   somethingElse();
}

そうすれば、どのスタイル（1、2、または3）を使用するかは実際には重要ではありません。これは、ifステートメントがテスト対象の条件を明確に記述しているためです。追加の構成は必要ありません。

重要なのは、コードをより明確にし、自己文書化することです。オブジェクト指向プログラミング言語を使用している場合は、オブジェクトを使用して状態（変数）を格納し、5〜10個のパラメーターを受け取るメソッドの作成を回避できます。

これらは同様の質問です：

ネストされたifを取り除くための最良の方法

このハイパーアイデンティティ化されたコードに代わるものはありますか

2番目のリンクは、恐ろしい全員のメンテナの悪夢を自己文書化コードに変換するためのより完全で複雑な方法を示しています。

これを変換する方法を示しています。

public String myFunc(SomeClass input)
{
    Object output = null;

    if(input != null)
    {
        SomeClass2 obj2 = input.getSomeClass2();
        if(obj2 != null)
        {
            SomeClass3 obj3 = obj2.getSomeClass3();
            if(obj3 != null && !BAD_OBJECT.equals(obj3.getSomeProperty()))
            {
                SomeClass4 = obj3.getSomeClass4();
                if(obj4 != null)
                {
                    int myVal = obj4.getSomeValue();
                    if(BAD_VALUE != myVal)
                    {
                        String message = this.getMessage(myVal);
                        if(MIN_VALUE <= message.length() &&
                           message.length() <= MAX_VALUE)
                        {
                            //now actually do stuff!
                            message = result_of_stuff_actually_done;
                        }
                    }
                }
            }
        }
    }
    return output;
}

これに：

if ( isValidInput() && 
    isRuleTwoReady() &&
    isRuleTreeDifferentOf( BAD_OBJECT ) &&
    isRuleFourDifferentOf( BAD_VALUE ) && 
    isMessageLengthInRenge( MIN_VALUE , MAX_VALUE ) ) { 
            message = resultOfStuffActuallyDone();
}

score 8 · Accepted Answer

私は最初のオプションを好みます。フェイルファストは、よりクリーンで明確で、読みやすく理解しやすいものです。

これが失敗ではないことは理解していますが、コンセプトは引き続き適用されます。ifネストされたステートメントはまったく好きではありません。

score 8 · Accepted Answer

メソッド機能のコントラクトを確実に満たすために、防御的なプログラミングを検討できます。

public void function(string value, string value2)
{
    if (string.IsNullOrEmpty(value1)) throw new ArgumentNullException("value1", "value 1 was not set");
    if (string.IsNullOrEmpty(value2)) throw new ArgumentNullException("value2", "value 2 was not set");

    DoSomething();
}

score 2 · Accepted Answer

読みやすく保守しやすいコードを書くための優れたヒューリスティックセットを提供するRobertC.Martinの本CleanCodeをお勧めします。

ここで、別のオプションは、条件を別のプライベート関数に抽出し、意図を説明するように名前を付けることです。提供されたコードは一般的であるため、あまりうまく機能しませんが、次のようになります。

public void function(string value, string value2)
{
    if (valuesAreValidAndEqual(value, value2))
    {
        DoSomething();
    }
}

private void valuesAreValidAndEqual(string value, string value2)
{
    return value != null && value2 == value;
}

明らかに、変数名と関数名がドメインに関連している場合、これはより便利です。

score 2 · Accepted Answer

それを独自の関数にリファクタリングします。ブール式の束よりも、説明的な関数名を読む方がはるかに優れています。

// leave complex conditional code out, so that we can focus on the larger problem in the function
public void function(string value, string value2)
{
    if (MyDescriptiveTestName)
    {
        DoSomething();
    }
}

// encapsulate complex conditional code so that we can focus solely on it.
private bool MyDescriptiveTestName(string value, string value2)
{
    if (value != null && value2 == value)
    {
        return true;
    }
    return false;
}

score 1 · Accepted Answer

多くの条件付きの関数がある場合は、switchif ではなくステートメントを使用します。可能であれば、詳細をいくつかの関数 (またはクラス) に分割することもあります。

関連するスタックオーバーフロー記事:

score 0 · Accepted Answer

関数内に非常に多くのステートメントがあるという事実は、関数をより小さなものに分割する必要があることを示しています。

if文を配置する最良の方法はありません。答えは主観的なものだと思います。

score 0 · Accepted Answer

コードの可読性について事前に考えているという事実は、戦いの半分です。

あなたの例のどれが最も読みやすいかに関しては、それはかなり主観的です。

与えられた例についての私の考え：

個人的には、最初の例が最もわかりやすいと思います。
ネストレベルや条件の数を最小限に抑えると、通常、読みやすさが向上します。
メソッドからの複数の出口点に反対する人もいますが（例1のように）、メソッドが非常に長くなる場合にのみ問題になると思います。入力をチェックしてすぐに失敗するだけであれば、それほど大したことではありません。

score 0 · Accepted Answer

私の好みは2番目のオプションです。個人的にそのようなコードを読むときは、ネストされた各レベルに入る条件を覚えています。最初の例では、最初の条件（value == nullがfalse）が引き続き保持されることをおそらく忘れます。3つ目も悪くはないですが、2つ目が好きです。

score 0 · Accepted Answer

明快さはしばしば判断するのが難しい品質です。あなたがコードを書いているときにあなたに明らかなことは、他の誰かにとって、あるいは十分な時間が経過した後のあなた自身にとってさえ、完全に鈍感であるかもしれません。

多くの条件付きチェックを使用して関数を作成するときの私の個人的な経験則は次のとおりです。

可能な場合は、条件を論理ユニットにグループ化します
読みやすさとデバッグを簡素化するために、明確に名前が付けられた中間値を使用してください。
同じ論理構造を複数回繰り返すことは避けてください（上記を参照）。
可能であれば、複雑または高価な条件を別々の関数にリファクタリングします。
可能であれば、メソッドの上部にある早期終了条件を使用して終了しますが、...
メソッドの本体全体にreturnステートメントをペッパーすることは避けてください。
演算子の優先順位に依存せず、括弧を使用してステートメントをグループ化します。
インデントに依存することを避け、ステートメントが1つしかない場合でもブロック（{...}）を使用します。これにより、保守性が向上します。

score 0 · Accepted Answer

私は 3 番目のオプションが好きですが、これは言語によって大きく異なります。3 番目のステートメントでは、ステートメントの最初の部分が失敗し、2 番目の部分が実行されないと想定しています。これは言語に依存します。ほとんどの C ベースの言語がこれを行うことは知っていますが、潜在的な問題である言語を指定しなかったためです。私が気付いていない、短絡の概念がないものがあるかもしれません。

coding-style - 関数内の if ステートメントで最善の方法は何ですか

11 に答える 11

Related

Reference