Есть ли лучший (или более чистый) способ написать следующий код?
if(conditionX)
{
if(condition1)
{
// code X1
}
else if(condition2)
{
// code X2
}
}
else if(conditionY)
{
if(condition1)
{
// code Y1
}
else if(condition2)
{
// code Y2
}
}
У меня есть еще несколько условий, но, думаю, вы понимаете.
Существует четыре подхода к этой проблеме, ни одна из которых не является универсальной:
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;
}
Если ваша проблема связана с чистым кодом с точки зрения просмотра источника, мой совет состоит в том, чтобы выделить блоки в свои разделы, например:
if (conditionX) processConditionX();
else if (conditionY) processConditionY();
и т.д.
Затем в подфункциях вы помещаете "мясо":
void processConditionX (void) {
if(condition1) {
// code X1
} else if(condition2) {
// code X2
}
}
Вы можете изменить его, чтобы передавать и возвращать параметры по мере необходимости, и я бы сделал имена условий и функций немного более наглядными, хотя я предполагаю, что они просто примеры здесь.
Вместо этого вы можете реализовать государственную машину:
#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);
}
Я бы предложил решение внутри первого, если в качестве параметра разделенные функции, которые затем решают, какой код выполнить, например:
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;
}
Лучше всего использовать полиморфизм (только если куски кода огромны)
Если это небольшие фрагменты кода, создание классов, очевидно, будет излишним.
Поэтому, если есть сходство во всех кодах, я предлагаю, казалось бы, легкую, но очень трудную задачу.
Всегда сложно создавать простые вещи.
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
}
}
Условие, которое вы можете параметризовать, должно храниться снаружи.
Надеюсь, что это поможет.
Я думаю, что этот способ может быть другим способом для решения вашего кода.
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;
}
Если сочетание условий означает что-то, я бы написал набор простых методов, возвращающих логические значения. У вас получилось бы что-то вроде:
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
}
Имена методов описывают условия. Не волнуйтесь, что методы вызываются только один раз (компилятор, вероятно, будет встраивать их в любом случае), что важно, чтобы ваш код затем сам документировался.
Меня удивляют другие предложенные ответы, которые в основном неправильны, если:
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);
исходный код не выглядит плохим. В зависимости от конкретного случая он может или не может быть более читаемым, чтобы сделать что-то вроде:
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
}