Как эффективный способ проверить кучу переменных, прежде чем двигаться дальше? - PullRequest
Новая
       10

Как эффективный способ проверить кучу переменных, прежде чем двигаться дальше?

1 голос
/ 07 сентября 2011

Я читал об идеальном размере методов и принципе единой ответственности, затем я посмотрел на некоторые из моего кода. Я чувствую, что могу разбить большую часть (> 90%) своих материалов на небольшие управляемые методы, но затем я приступаю к проверке данных или формы. Это всегда кажется действительно большим и раздутым. Я склонен проверять свои данные с помощью вложенных операторов if и стараться выявлять ошибки или проблемы на каждом уровне. Но когда я начинаю получать 6, 8, 10+ уровней проверки, это очень громоздко. Но я не уверен, как разбить это, чтобы быть более эффективным.

Пример чего-то, что я считаю громоздким, но не уверен, как его улучшить, приведен ниже. Каждый из уровней имеет уникальное действие, связанное с ним, и только когда все условия возвращают true, все может вернуть true, но это трудно прочитать, особенно после возвращения в программу через месяц или около того. 

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");
}

Ответы [ 7 ]

1 голос
/ 07 сентября 2011

Если ваша основная цель - разбить методы на управляемые куски, вы можете инкапсулировать каждый блок 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;
}
1 голос
/ 07 сентября 2011

Что-то похожее на 

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

Так что каждое условие не является вложенным

0 голосов
/ 07 сентября 2011

Есть несколько способов справиться с этим.Вы действительно хотите ограничить количество повторяющегося кода, такого как код, который добавляет выходное сообщение, которое почти идентично в четырех или более местах.

Если вы думаете об этих вложенных if…else блоках как о последовательностигде в случае неудачи вы предпринимаете действия и прекращаете дальнейшую обработку, вы можете создать список и использовать функциональность FirstOrDefault LINQ для последовательной обработки списка условий до тех пор, пока не произойдет сбой, или вы получите null, если все они пройдут.1006 *

Создание объекта для инкапсуляции условий поможет консолидировать и уменьшить дублирование.

Вот пример: 

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
}
0 голосов
/ 07 сентября 2011

Я бы попытался определить каждую валидацию как предикат, что-то вроде этого ... 

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

Тогда вы могли бы связать столько их вместе, сколько вам нужно.

0 голосов
/ 07 сентября 2011

Одним из способов является использование метода проверки, который вызывается до выполнения другого кода.

Например: 

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.
}
0 голосов
/ 07 сентября 2011

Обратный поток.Вместо 

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;
0 голосов
/ 07 сентября 2011

Вы можете инвертировать свои операторы if и использовать вместо них пункты охраны. Смотрите этот пример .

...