Как сделать большой if-оператор более читабельным - PullRequest
5 голосов
/ 22 июля 2010

Есть ли лучший способ написать условие с большим количеством проверок AND, чем большой оператор IF с точки зрения ясности кода?

Например, в настоящее время мне нужно сделать поле на экране обязательным, если другие поля не соответствуют определенным требованиям. На данный момент у меня есть оператор IF, который превышает 30 LOC, и это просто не выглядит правильным.

if(!(field1 == field2 &&
     field3 == field4 &&
     field5 == field6 &&
     .
     .
     .
     field100 == field101))
{
   // Perform operations
}

Является ли решение просто разбить их на более мелкие порции и присвоить результаты меньшему количеству логических переменных? Каков наилучший способ сделать код более читабельным?

Спасибо

Ответы [ 9 ]

9 голосов
/ 22 июля 2010

Я бы рассмотрел создание правил в форме предиката:

bool FieldIsValid() { // condition }
bool SomethingElseHappened() { // condition }
// etc

Тогда я бы сам создал список этих предикатов:

IList<Func<bool>> validConditions = new List<Func<bool>> {
  FieldIsValid,
  SomethingElseHappend,
  // etc
};

Наконец, я бы написал условие для выдвижения условий:

if(validConditions.All(c => c())
{
  // Perform operations
}
2 голосов
/ 22 июля 2010

Правильный подход зависит от деталей, которые вы не предоставили.Если сравниваемые элементы могут быть выбраны по какому-либо индексу (числовой счетчик, список идентификаторов полей и т. Д.), Вам, вероятно, лучше всего это сделать.Например, что-то вроде:

Ok = True
For Each fld as KeyValuePair(Of Control, String) in CheckFields
  If fld.FormField.Text  fld.RequiredValue Then
    OK = False
    Exit For
  End If
Next

Создание списка элементов управления и строк может быть небольшим неудобством, но есть разумные способы сделать это.

1 голос
/ 22 июля 2010

Может быть полезно начать использовать Workflow Engine для C #.Он был специально разработан для графической компоновки таких сложных алгоритмов принятия решений.

Windows WorkFlow Foundation

1 голос
/ 22 июля 2010

Часть проблемы в том, что вы смешиваете метаданные и логику.

Какие вопросы требуются (/ должны быть равны / минимальная длина / и т.д.) - это метаданные.

Проверка того, что каждое поле соответствует его требованиям, является программной логикой.

Список требований (и поля, которые также применяются) должны храниться где-то еще, а не внутри большого оператора if.

Тогда ваша логика проверки читает список, проходит по нему и сохраняет промежуточный итог. Если ЛЮБОЕ поле не заполнено, вы должны предупредить пользователя.

1 голос
/ 22 июля 2010

Первое, что я бы изменил для удобочитаемости, - это убрать почти скрытое отрицание, инвертировав утверждение (используя законы де Моргана ):

if ( field1 != field2 || field3 != field4 .... etc )
{
   // Perform operations
}

Хотя используется серия &&, а не || есть небольшое улучшение производительности, я чувствую, что отсутствие читабельности исходного кода стоит изменений.

Если бы производительность была проблемой, вы могли бы разбить операторы на серию операторов if, но к тому времени это станет беспорядком!

1 голос
/ 22 июля 2010

Вы можете преобразовать свое условие в отдельную функцию, а также использовать Законы де Моргана , чтобы немного упростить вашу логику.

Также - ваши переменные действительно все называются fieldN?

1 голос
/ 22 июля 2010

Лично я чувствую, что разбиение этого на куски только сделает общее утверждение менее ясным.Это сделает код длиннее, а не более кратким.

Я бы, вероятно, преобразовал эту проверку в метод класса, чтобы вы могли повторно использовать его по мере необходимости и тестировать в одном месте.Однако я бы, скорее всего, оставил чек в том виде, в каком он у вас, - один оператор if с множеством условий, по одному на строку.

0 голосов
/ 22 июля 2010

Разве это не то, для чего arrays в основном?

Вместо 100 variables с именем fieldn создайте array из 100 values.

Тогда у вас может быть функция для loop комбинаций в массиве и return true or false, если условие соответствует.

0 голосов
/ 22 июля 2010

Есть ли какие-то другие отношения между всеми сравниваемыми переменными, которые вы можете использовать?

Например, все ли они являются членами двух классов?

Если это так, и если ваши требования к производительности не исключают этого, вы можете собрать все ссылки на них в список или массив, а затем сравнить их в цикле. Иногда вы можете сделать это при построении объекта, а не при каждом сравнении.

Мне кажется, что настоящая проблема в архитектуре, а не в выражении if () - конечно, это не значит, что ее легко исправить, я ценю это.

Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...