Вложенные избыточные условия "если"

Есть ли лучший (или более чистый) способ написать следующий код?

if(conditionX)
{
    if(condition1)
    {
        // code X1
    }
    else if(condition2)
    {
        // code X2
    }
}
else if(conditionY)
{
    if(condition1)
    {
        // code Y1
    }
    else if(condition2)
    {
        // code Y2
    }
}

У меня есть еще несколько условий, но, думаю, вы понимаете.

Ответы

Ответ 1

Существует четыре подхода к этой проблеме, ни одна из которых не является универсальной:

Оставить все как есть. Здесь не так много дубликатов кода. Если вычисления condition1 и condition2 сложны, вычислите их заранее и сохраните их в bool переменных
Сделайте conditionX и conditionY результат, который позволяет объединить condition1 и condition2. Это не всегда возможно, но в некоторых ситуациях вы можете подготовить переменную, которая объединяет действия, предпринятые в двух ветвях, скажем, с помощью указателя функции или лямбда.
Поместите логику обработки в подклассы с виртуальными функциями, чтобы устранить условную логику. Это возможно только тогда, когда ваш первоначальный дизайн упустил возможность подкласса. По сути, этот подход подталкивает решение к conditionX/conditionY в место, где создается подкласс, а затем "повторно использует" это решение позже, вызывая надлежащее переопределение виртуальной функции в интерфейсе.
Создать числовую комбинацию, представляющую все три условия, и преобразовать в switch. Этот трюк объединяет условные обозначения, уменьшая вложенность.

Вот пример последнего подхода:

int caseNumber = ((conditionX?1:0) << 3)
               | ((conditionY?1:0) << 2)
               | ((condition2?1:0) << 1)
               | ((condition1?1:0) << 0);
switch (caseNumber) {
    case 0x09:
    case 0x0D:
    case 0x0F: // code X1
        break;
    case 0x0A:
    case 0x0E: // code X2
        break;
    case 0x05:
    case 0x07: // code Y1
        break;
    case 0x06: // code Y2
        break;
}

Ответ 2

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

if      (conditionX) processConditionX();
else if (conditionY) processConditionY();

и т.д.

Затем в подфункциях вы помещаете "мясо":

void processConditionX (void) {
    if(condition1) {
        // code X1
    } else if(condition2) {
        // code X2
    }
}

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

Ответ 3

Вместо этого вы можете реализовать государственную машину:

#define COMBINATION(a,b,c,d) (((a)<<3)|((b)<<2)|((c)<<1)|((d)<<0))

switch (COMBINATION(conditionX,conditionY,condition1,condition2))
{
    case COMBINATION(0,0,0,0):           break;
    case COMBINATION(0,0,0,1):           break;
    case COMBINATION(0,0,1,0):           break;
    case COMBINATION(0,0,1,1):           break;
    case COMBINATION(0,1,0,0):           break;
    case COMBINATION(0,1,0,1): CodeY2(); break;
    case COMBINATION(0,1,1,0): CodeY1(); break;
    case COMBINATION(0,1,1,1): CodeY1(); break;
    case COMBINATION(1,0,0,0):           break;
    case COMBINATION(1,0,0,1): CodeX2(); break;
    case COMBINATION(1,0,1,0): CodeX1(); break;
    case COMBINATION(1,0,1,1): CodeX1(); break;
    case COMBINATION(1,1,0,0):           break;
    case COMBINATION(1,1,0,1): CodeX2(); break;
    case COMBINATION(1,1,1,0): CodeX1(); break;
    case COMBINATION(1,1,1,1): CodeX1(); break;
}

Это включает только одну операцию ветвления, поэтому она может быть немного более эффективной (хотя она также включает в себя дополнительное вычисление времени выполнения (в строке switch)).

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

Обратите внимание, что если любой переменных условия может иметь значение, отличное от 1 или 0, то вы должны:

#define COMBINATION(a,b,c,d) (((a)?8:0)|((b)?4:0)|((c)?2:0)|((d)?1:0))

Обновление (приписывается @Jonathan Wakely в одном из комментариев ниже):

Если вы используете С++ 11, вы можете заменить макрос COMBINATION на функцию constexpr:

constexpr int COMBINATION(bool a,bool b,bool c,bool d)
{
    return ((int)a<<3) | ((int)b<<2) | ((int)c<<1) | ((int)d<<0);
}

Ответ 4

Я бы предложил решение внутри первого, если в качестве параметра разделенные функции, которые затем решают, какой код выполнить, например:

if(conditionX)
{
    Method1(Condition Parameters)
}
else if(conditionY)
{
    Method1(Condition Parameters)
}

Другой способ - предоставить всю необходимую информацию методу решения (матрице), этот метод возвращает целое число, которое вы используете в инструкции switch, чтобы решить, какой код выполнить. Таким образом, вы разделяете логику desicion, которая делает ее доступной для чтения и удобной при необходимости:

DecisionMatrix(conditionX, conditionY, condition1, condition2)
{
  //  return a value according to the conditions for Example:
  // CoditionX + Condition1 => return 1
  // CoditionX + Condition2 => return 2
  // CoditionY + Condition1 => return 3
  // CoditionY + Condition2 => return 4
}

switch(DecisionMatrix)
{
    case 1: //run code X1       
    break;
    case 2: //run code X2
    break;
    case 3: //run code Y1       
    break;
    case 4: //run code Y2
    break;
}

Ответ 5

Лучше всего использовать полиморфизм (только если куски кода огромны)
Если это небольшие фрагменты кода, создание классов, очевидно, будет излишним.

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

Всегда сложно создавать простые вещи.

if (conditionX) {
    method(parameterX);
else if (conditionY) {
    method(parameterY);
}

где

void method(ParameterType e) {
    if (condition 1) {
        // Code in terms of parameter e
    } else if (condition2) {
        // Code in terms of parameter e
    }
}

Условие, которое вы можете параметризовать, должно храниться снаружи.

Надеюсь, что это поможет.

Ответ 6

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

enum ConditionParentType
{
    CONDITION_NONE = 0,
    CONDITION_X,
    CONDITION_Y,
};

enum ConditionChildType
{
    CONDITION_0 = 0,
    CONDITION_1,
    CONDITION_2,
};

class ConditionHandler
{
public:
    explicit ConditionHandler(ConditionParentType p_type, ConditionChildType c_type) 
        : p_type_(p_type), c_type_(c_type) {};
    void DoAction()
    {
        if(child_type == CONDITION_1)
        {

        }
        else if(child_type == CONDITION_2)
        {

        }
        else
        {
            //error
        }
    }

private:
    const ConditionParentType p_type_;
    const ConditionChildType  c_type_;
}; 

int main(int argc, char *argv[]) 
{
    ConditionParentType parent_type = GetParentType();
    ConditionChildType  child_type  = GetChildType();

    ConditionHandler handler(parent_type, child_type);
    handler.DoAction();

    getchar();

    return 0;
}

Ответ 7

Если сочетание условий означает что-то, я бы написал набор простых методов, возвращающих логические значения. У вас получилось бы что-то вроде:

  if (first-condition(conditionX, condition1)) {
    // code X1
  } else if (first-condition(conditionX, condition2)) {
    // code X2
  } else if (third-condition(conditionY, condition1)) {
    // code Y1
  } else if (fourth-condition(conditionY, condition2)) {
    // code Y2
  }

Имена методов описывают условия. Не волнуйтесь, что методы вызываются только один раз (компилятор, вероятно, будет встраивать их в любом случае), что важно, чтобы ваш код затем сам документировался.

Ответ 8

Меня удивляют другие предложенные ответы, которые в основном неправильны, если:

Два повторяющихся условия condition1 или condition2 являются сложными, и в этом случае DRY входит в игру или
Любое из четырех условий имеет побочные эффекты, или
Любое из условий медленное (например, найдите минимум большого массива или прочитайте файл) или
Требуется булевское короткое замыкание, например: if (p == 0) {...} else if (p->foo == 42) {...}.

Если ни одна из них не удерживается, как и в случае с 99,42% времени, оставьте код таким, какой он есть. Или, как незначительное изменение, измените его так, чтобы вложенность (то есть отступы) была только одним уровнем, а не двумя.

В противном случае вам нужно будет использовать временные переменные следующим образом

const bool tstX =         (conditionX);    
const bool tstY = tstX || (conditionY);    
const bool tst1 = tstY && (condition1);
const bool tst2 = tstY && !tst1 && (condition2);

Ответ 9

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

if(conditionX and condition1) {
  // code X1
} 
else if(conditionX and condition2) {
  // code X2
}
else if(conditionY and condition1) {
  // code Y1
}
else if(conditionY and condition2)
  // code Y2
}