1

メソッドの理想的なサイズと単一責任の原則について読んだ後、自分のコードをいくつか見ていきます。私は自分のものの多く (>90%) を小さな管理しやすいメソッドに分割できると感じていますが、その後、データまたはフォームの検証に取りかかります。それはいつも本当に大きくて肥大しているように見えます。入れ子になった if ステートメントを使用してデータを検証し、各レベルでエラーや問題を見つけようとする傾向があります。しかし、6 レベル、8 レベル、10 レベル以上の検証を取得し始めると、非常に面倒です。しかし、それをより効果的に分割する方法がわかりません。

私が面倒だと思うが、それを改善する方法がわからないものの例を以下に示します。各レベルには固有のアクションが関連付けられており、すべての条件が true に戻った場合にのみ全体が戻ることができますがtrue、特に 1 か月ほど後にプログラムに戻った後では、これを読むのは困難です。

if (InitialUsageSettings.zeroed || sender.Equals(btnZero))
{   
    if (InitialUsageSettings.StandardFilterRun || sender.Equals(btnStandard))
    {   
        if (InitialUsageSettings.ReferenceFilterRun || sender.Equals(btnReference) || sender.Equals(btnStandard))
        {   
            if (InitialUsageSettings.PrecisionTestRun || sender.Equals(btnPrecision) || sender.Equals(btnReference) || sender.Equals(btnStandard))
            {   
                if (txtOperatorID.Text.Length > 0 && cboProject.Text.Length > 0 && cboFilterType.Text.Length > 0 && cboInstType.Text.Length > 0)
                {   
                    if (txtFilterID.Text.Length > 0 && txtLot.Text.Length > 0)
                    {   
                        return true;
                    }
                    else
                    {
                        if (txtFilterID.Text.Length == 0)
                        {
                            //E
                        }
                        if (txtLot.Text.Length == 0)
                        {
                            //D
                        }
                    }
                }
                else
                {
                    if (txtOperatorID.Text.Length == 0)
                    {
    //A
                    }
                    if (cboProject.Text.Length == 0)
                    {
    //B
                    }
                    if (cboFilterType.Text.Length == 0)
                    {
    //C
                    }
                    if (cboInstType.Text.Length == 0)
                    {
    //D
                    }
                    //return false;
                }
            }
            else
            {
                outputMessages.AppendLine("Please correct the folloring issues before taking a reading: X");
            }
        }
        else
        {
            outputMessages.AppendLine("Please correct the folloring issues before taking a reading: Y");
        }
    }
    else
    {
        outputMessages.AppendLine("Please correct the folloring issues before taking a reading: Z");
    }
}
else
{
    outputMessages.AppendLine("Please correct the folloring issues before taking a reading: A");
}
4

7 に答える 7

1

似たようなもの

if(errorCondition1)
  errors.add(message1);
if(errorCondition2)
  errors.add(message2);
return errors.Count == 0;

したがって、各条件はネストされていません

于 2011-09-07T13:52:29.347 に答える
1

メソッドを扱いやすいチャンクに分割することが主な目的である場合は、各ifブロックを独自のメソッドにカプセル化できます。例えば：

 if (InitialUsageSettings.zeroed || sender.Equals(btnZero))
 {
     ValidateStandardFilter();
 }
 else
 {   
     outputMessages.AppendLine("Please correct the folloring issues before taking a reading: A");
 }

しかし、このメソッドには責任が多すぎるように思えます。検証してメッセージを出力しようとしています。代わりに、メソッドは検証のみを担当する必要があります。

public ValidationResult Validate(Sender sender)
{
    if (!(InitialUsageSettings.zeroed || sender.Equals(btnZero)))
    {   
        return ValidationResult.Error("A");
    }
    if (!(InitialUsageSettings.StandardFilterRun || sender.Equals(btnStandard)))
    {   
        return ValidationResult.Error("Z");
    }
    // Etc...
    if (txtOperatorID.Text.Length == 0)
    {
        errors.Add("A");
    }
    if (cboProject.Text.Length == 0)
    {
        errors.Add("B");
    }
    if (cboFilterType.Text.Length == 0)
    {
        errors.Add("C");
    }
    if (cboInstType.Text.Length == 0)
    {
        errors.Add("D");
    }
    if(errors.Count > 0)
    {
        return ValidationResult.Errors(errors);
    }
    if (txtFilterID.Text.Length == 0)
    {
        errors.Add("E");
    }
    if (txtLot.Text.Length == 0)
    {
        errors.Add("D");
    }
    return errors.Count > 0 
        ? ValidationResult.Errors(errors) 
        : ValidationResult.Success();
}

そして、呼び出しコードは出力について心配することができます:

var result = Validate(sender);
if (result.IsError)
{
    outputMessages.AppendLine("Please correct...: " + result.Issue);
}

ValidationResultクラスがどのように見えるかを理解するには、ここで私の答えを参照してください。

アップデート

上記のコードをさらにリファクタリングして、繰り返しをさらに減らすことができます。

public ValidationResult Validate(Sender sender)
{
    if (!(InitialUsageSettings.zeroed || sender.Equals(btnZero)))
    {   
        return ValidationResult.Error("A");
    }
    if (!(InitialUsageSettings.StandardFilterRun || sender.Equals(btnStandard)))
    {   
        return ValidationResult.Error("Z");
    }
    // Etc...
    
    var firstErrorBatch = GetEmptyStringErrors(
        new[]{
            new InputCheckPair(txtOperatorID, "A"),
            new InputCheckPair(cboProject, "B"),
            new InputCheckPair(cboFilterType, "C"),
            new InputCheckPair(cboInstType, "D"),
        })
        .ToList();
    if(firstErrorBatch.Count > 0)
    {
        return ValidationResult.Errors(firstErrorBatch);
    }
        
    var secondErrorBatch = GetEmptyStringErrors(
        new[]{
            new InputCheckPair(txtFilterID, "E"),
            new InputCheckPair(txtLot, "D"),
        })
        .ToList();
    return secondErrorBatch.Count > 0 
        ? ValidationResult.Errors(secondErrorBatch) 
        : ValidationResult.Success();
}

private class InputCheckPair
{
    public InputCheckPair(TextBox input, string errorIfEmpty)
    {
        Input = input;
        ErrorIfEmpty = errorIfEmpty;
    }
    public TextBox Input {get; private set;}
    public string ErrorIfEmpty{get; private set;}
}

public IEnumerable<string> GetEmptyStringErrors(IEnumerable<InputCheckPair> pairs)
{
    return from p in pairs where p.Input.Text.Length == 0 select p.ErrorIfEmpty;
}
于 2011-09-07T13:54:55.510 に答える
0

1つの方法は、他のコードを実行する前に呼び出される検証メソッドを用意することです。

例えば：

private String ValidateThis() {
  StringBuilder result = new StringBuilder();

  if (!cond1) {
    result.AppendLine("error on cond1");
  } 

   if (!cond2) {
     result.AppendLine("error on cond2");
   }

   return result.ToString();
}

public void ButtonClick(object sender) {
    String isValid = ValidateThis();

    if (!String.IsNullOrEmpty(isValid)) {
        // set your error message
        outputMessages.AppendLine(isValid);
        return;
    } 
    // ... perform your other operations.
}
于 2011-09-07T13:56:52.983 に答える
0

これに対処する方法はいくつかあります。出力メッセージを追加するコードなど、4 つ以上の場所でほぼ同じコードの繰り返しの量を制限する必要があります。

これらのネストされたブロックをシーケンスと考えると、if…else1 つが失敗するとすぐにアクションを実行し、それ以上の処理を停止します。リストを作成し、LINQ のFirstOrDefault機能を活用して、条件のリストを 1 つが失敗するまで順番に処理するか、条件が失敗したかどうかを取得しますnull。すべて合格。

条件をカプセル化するオブジェクトを作成すると、重複を統合して削減するのに役立ちます。

次に例を示します。

public class Validator
{
    public Validator(string code, bool settingsCheck, Button button, object sender)
    {
        Code = code;
        IsValid = sender != null && button != null && sender.Equals(button);
    }

    public bool IsValid { get; private set; }

    public string Code { get; private set; }
}

これで、メソッドは次のようになります。

var validationChecks = new List<Validator>
{
    new Validator("A", InitialUsageSettings.zeroed, btnZero, sender),
    new Validator("Z", InitialUsageSettings.StandardFilterRun, btnStandard, sender),
    new Validator("Y", InitialUsageSettings.ReferenceFilterRun, btnReference, sender),
    new Validator("X", InitialUsageSettings.PrecisionTestRun, btnPrecision, sender)
}

var failure = validationChecks.FirstOrDefault(check => !check.IsValid);
if (failure != null)
{
    outputMessages.AppendLineFormat(
        "Please correct the following issues before taking a reading: {0}", failure.Code);
    return;
}
else
{
    // further checks; I'm not sure what you're doing there with A-E
}
于 2011-09-07T14:42:17.833 に答える
0

私は、各検証を述語として定義しようとします。このようなものです...

delegate bool Validator(object sender, out string message);

次に、必要な数だけそれらをつなぎ合わせることができます。

于 2011-09-07T14:03:19.270 に答える
0

if ステートメントを逆にして、代わりにガード句を使用できます。この例を参照してください。

于 2011-09-07T13:53:36.897 に答える
0

流れを逆にする。それ以外の

If(cond) {
  if(someothercond) {
     //great sucess!
     return true;
  } else {
    // handle
    return false;
  }
} else {
  // handle
  return false;
}

行う：

if(!cond1) {
  // handle
  return false;
}
if(!someothercond) {
  // handle
  return false;
}

// great sucess!
return true;
于 2011-09-07T13:53:52.390 に答える