Как вы справляетесь с огромными if-условиями? - PullRequest
28 голосов
/ 08 августа 2008

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

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

Пример, все в одной строке:

if (var1 = true && var2 = true && var2 = true && var3 = true && var4 = true && var5 = true && var6 = true)
{

Пример, многострочный:

if (var1 = true && var2 = true && var2 = true
 && var3 = true && var4 = true && var5 = true
 && var6 = true)
{

Пример вложенный:

if (var1 = true && var2 = true && var2 = true && var3 = true)
{
     if (var4 = true && var5 = true && var6 = true)
     {

Ответы [ 21 ]

61 голосов
/ 08 августа 2008

Разделите условие на несколько логических значений, а затем используйте мастер-логическое значение в качестве условия.

bool isOpaque = object.Alpha == 1.0f;
bool isDrawable = object.CanDraw && object.Layer == currentLayer;
bool isHidden = hideList.Find(object);

bool isVisible = isOpaque && isDrawable && ! isHidden;

if(isVisible)
{
    // ...
}

Еще лучше:

public bool IsVisible {
    get
    {
        bool isOpaque = object.Alpha == 1.0f;
        bool isDrawable = object.CanDraw && object.Layer == currentLayer;
        bool isHidden = hideList.Find(object);

        return isOpaque && isDrawable && ! isHidden;
    }
}

void Draw()
{
     if(IsVisible)
     {
         // ...
     }
}

Убедитесь, что вы даете своим переменным имя, которое фактически указывает на намерение, а не на функцию. Это очень поможет разработчику поддерживать ваш код ... это может быть ВЫ!

12 голосов
/ 08 августа 2008

Я удивлен, что никто еще не получил это. Для этого типа проблемы есть рефакторинг:

http://www.refactoring.com/catalog/decomposeConditional.html

7 голосов
/ 08 августа 2008

Здесь есть две проблемы: удобочитаемость и понятность

Решение «удобочитаемости» - это вопрос стиля, и поэтому оно открыто для интерпретации. Мои предпочтения таковы:

if (var1 == true && // Explanation of the check
    var2 == true && // Explanation of the check
    var3 == true && // Explanation of the check
    var4 == true && // Explanation of the check
    var5 == true && // Explanation of the check
    var6 == true)   // Explanation of the check
    { }

или это:

if (var1 && // Explanation of the check
    var2 && // Explanation of the check
    var3 && // Explanation of the check
    var4 && // Explanation of the check
    var5 && // Explanation of the check
    var6)   // Explanation of the check
    { }

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

/// <Summary>
/// Tests whether all the conditions are appropriately met
/// </Summary>
private bool AreAllConditionsMet (
    bool var1,
    bool var2,
    bool var3,
    bool var4,
    bool var5,
    bool var6)
{
    return (
        var1 && // Explanation of the check
        var2 && // Explanation of the check
        var3 && // Explanation of the check
        var4 && // Explanation of the check
        var5 && // Explanation of the check
        var6);  // Explanation of the check
}

private void SomeMethod()
{
    // Do some stuff (including declare the required variables)
    if (AreAllConditionsMet (var1, var2, var3, var4, var5, var6))
    {
        // Do something
    }
}

Теперь при визуальном сканировании метода «SomeMethod» реальная сложность тестовой логики скрыта, но семантическое значение сохраняется для понимания людьми на высоком уровне. Если разработчику действительно необходимо понять детали, можно проверить метод AreAllConditionsMet.

Это формально известно как шаблон рефакторинга «Разобрать условно», я думаю. Такие инструменты, как Resharper или Refactor Pro! может сделать процесс рефакторинга таким простым!

Во всех случаях ключом к читаемому и понятному коду является использование реалистичных имен переменных. Хотя я понимаю, что это надуманный пример, «var1», «var2» и т. Д. не допустимые имена переменных. У них должно быть имя, которое отражает основную природу данных, которые они представляют.

6 голосов
/ 08 августа 2008

Я буду часто разбивать их на булевы переменные компонента:

bool orderValid = orderDate < DateTime.Now && orderStatus != Status.Canceled;
bool custValid = customerBalance == 0 && customerName != "Mike";
if (orderValid && custValid)
{
...
5 голосов
/ 08 августа 2008

Во-первых, я бы удалил все == true детали, которые сделали бы его на 50% короче;)

Когда у меня большое состояние, я ищу причины. Иногда я вижу, что должен использовать полиморфизм, иногда мне нужно добавить какой-нибудь объект состояния. По сути, это подразумевает необходимость рефакторинга (запах кода).

Иногда я использую законы Де-Моргана , чтобы немного упростить логические выражения.

4 голосов
/ 08 августа 2008

Проверьте Шаблоны реализации от Кента Бека. Существует определенная схема, о которой я думаю, которая может помочь в этой ситуации ... она называется «Страж». Вместо того, чтобы иметь кучу условий, вы можете разбить их на охрану, что дает понять, какие условия являются неблагоприятными в методе.

Так, например, если у вас есть метод, который что-то делает, но есть определенные условия, когда он не должен что-то делать, вместо:

public void doSomething() {
    if (condition1 && condition2 && condition3 && condition4) {
        // do something
    }
}

Вы можете изменить его на:

public void doSomething() {
    if (!condition1) {
        return;
    }

    if (!condition2) {
        return;
    }

    if (!condition3) {
        return;
    }

    if (!condition4) {
        return;
    }

    // do something
}

Это немного более многословно, но гораздо более читабельно, особенно когда у вас начинается странное вложение, охранник может помочь (в сочетании с методами извлечения).

Кстати, я очень рекомендую эту книгу.

3 голосов
/ 08 августа 2008

Я видел много людей и редакторов, которые делали отступы в каждом условии в вашем операторе if с одной вкладкой или сравнивали его с открытым паренем:

if (var1 == true
    && var2 == true
    && var3 == true
   ) {
    /* do something.. */
}

Обычно я ставлю закрывающую скобку в ту же строку, что и последнее условие:

if (var1 == true
    && var2 == true
    && var3 == true) {
    /* do something.. */
}

Но я не думаю, что это так же чисто.

2 голосов
/ 18 августа 2008

Совет Стива Макконелла от Код завершен Используйте многомерную таблицу. Каждая переменная служит индексом для таблицы, и оператор if превращается в поиск в таблице. Например, если (размер == 3 && вес> 70) переводит в решение записи таблицы [размер] [весовая группа]

2 голосов
/ 18 августа 2008

Попробуйте посмотреть на функторы и предикаты. Проект Apache Commons имеет большой набор объектов, позволяющих вам инкапсулировать условную логику в объекты. Пример их использования доступен на O'reilly здесь . Выдержка из примера кода:

import org.apache.commons.collections.ClosureUtils;
import org.apache.commons.collections.CollectionUtils;
import org.apache.commons.collections.functors.NOPClosure;

Map predicateMap = new HashMap();

predicateMap.put( isHonorRoll, addToHonorRoll );
predicateMap.put( isProblem, flagForAttention );
predicateMap.put( null, ClosureUtils.nopClosure() );

Closure processStudents = 
    ClosureUtils.switchClosure( predicateMap );

CollectionUtils.forAllDo( allStudents, processStudents );

Теперь детали всех этих предикатов isHonorRoll и замыканий, используемых для их оценки:

import org.apache.commons.collections.Closure;
import org.apache.commons.collections.Predicate;

// Anonymous Predicate that decides if a student 
// has made the honor roll.
Predicate isHonorRoll = new Predicate() {
  public boolean evaluate(Object object) {
    Student s = (Student) object;

    return( ( s.getGrade().equals( "A" ) ) ||
            ( s.getGrade().equals( "B" ) && 
              s.getAttendance() == PERFECT ) );
  }
};

// Anonymous Predicate that decides if a student
// has a problem.
Predicate isProblem = new Predicate() {
  public boolean evaluate(Object object) {
    Student s = (Student) object;

    return ( ( s.getGrade().equals( "D" ) || 
               s.getGrade().equals( "F" ) ) ||
             s.getStatus() == SUSPENDED );
  }
};

// Anonymous Closure that adds a student to the 
// honor roll
Closure addToHonorRoll = new Closure() {
  public void execute(Object object) {
    Student s = (Student) object;

    // Add an award to student record
    s.addAward( "honor roll", 2005 );
    Database.saveStudent( s );
  }
};

// Anonymous Closure flags a student for attention
Closure flagForAttention = new Closure() {
  public void execute(Object object) {
    Student s = (Student) object;

    // Flag student for special attention
    s.addNote( "talk to student", 2005 );
    s.addNote( "meeting with parents", 2005 );
    Database.saveStudent( s );
  }
};
2 голосов
/ 08 августа 2008

Ну, во-первых, почему бы и нет:

if (var1 && var2 && var2 && var3 && var4 && var5 && var6) {
...

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

Не лучше, но то, что я делал в прошлом: (Следующий метод предотвращает короткое замыкание логического тестирования, все тесты запускаются, даже если первый неверен. Шаблон не рекомендуется, если вы не знаете, что нужно всегда выполнять весь код перед возвратом - спасибо ptomato за обнаружение моей ошибки!)

логическое ok = cond1;
хорошо & = cond2;
хорошо & = cond3;
хорошо & = cond4;
хорошо & = cond5;
хорошо & = cond6;

Что совпадает с: (не то же самое, см. Примечание выше!)

ok = (cond1 && cond2 && cond3 && cond4 && cond5 && cond6);

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