Цепочка условий OR в выражении if

В моем коде есть оператор if, который выглядит так:

if(someFunction1(a) || someFunction2(b->b1,c) || *d == null || somefunction3(e) > f * g || !e->e1 || ...){
   return 0;
} else {
   do_something;
}

В моем коде с реальными именами переменных и функций есть условия почти в трех строках, и это выглядит очень недооцененным. Поэтому я решил переписать его в форму:

if(someFunction1(a)){
   return 0;
} else if(someFunction2(b->b1,c)){
   return 0;
} else if(*d == null){
   return 0;
} else if(somefunction3(e) > f * g){
   return 0;
} else if(!e->e1){
   return 0;
} else if(...){
   return 0;
} else{
   do_something;
}

Есть ли аргумент, почему я не должен этого делать?

Ответ 1

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

if( someFunction1(a)
 || someFunction2(b->b1,c)
 || *d == null
 || somefunction3(e) > f * g
 || !e->e1
 || ...
){
   return 0;
} else {
   do_something;
}

Вы видите, как красиво все выстраивается? Это действительно похоже на пробирку, с которой программа падает до тех пор, пока она не достигнет определенного состояния. И если у вас есть &&, это выглядит как цепочка операций, которые не должны быть разбиты.

Ответ 2

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

bool unnecessary = someFunction1(a) || someFunction2(b->b1,c);
bool beyondTolerance = somefunction3(e) > f * g;
bool invalidInput = *d == nullptr || !e->e1;

if (unnecessary || beyondTolerance || invalidInput)
    return 0;
else 
    ...

Это рефинансирование "Декомпозиция условного" Мартина Фаулера .

Ответ 3

Вариант 1:

лаконичность
Одна точка выхода, чтобы избежать избыточности оператора return.

Вариант 2:

Точная точка отказа может быть легко диагностирована. Журналы могут быть добавлены в каждую ветвь для обнаружения сбоя.

Ответ 4

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

Но эта избыточность не возникает в первой реализации.

Оба варианта аналогичны.

Ответ 5

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

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

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

С учетом вышеизложенного лучше всего написать такой вариант:

/* comments here */
if(someFunction1(a)){
   return 0;
} 

/* comments here */
if(someFunction2(b->b1,c)){
 return 0;
}

...

/* if we got here, then there are no errors */
do_something();

Это также имеет преимущество в обслуживании, если вам нужно выполнить код между проверками ошибок. Или если вы хотите разбить некоторые более сложные выражения на несколько строк для удобства чтения.

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

Ответ 6

Вы можете сделать это следующим образом

int not_valid = someFunction1(a) || 
                someFunction2(b->b1,c) || 
                *d == null || 
                somefunction3(e) > f * g || 
                !e->e1 || ...;

if ( !not_valid )
{
   do_something;
}

return !not_valid;

Вместо not_valid вы можете выбрать более подходящее имя.:)