оператор if - оценка короткого замыкания vs удобочитаемость

90

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

например, это:

if (SomeComplicatedFunctionCall() || OtherComplicatedFunctionCall())
{
    // do stuff
}

в это

bool b1 = SomeComplicatedFunctionCall();
bool b2 = OtherComplicatedFunctionCall();

if (b1 || b2)
{
    //do stuff
}

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

Но с этим извлечением я потерял оценку короткого замыкания (SCE).

  1. Неужели я каждый раз теряю SCE? Есть ли сценарий, при котором компилятору разрешено «оптимизировать его» и по-прежнему предоставлять SCE?
  2. Есть ли способы сохранить улучшенную читаемость второго фрагмента без потери SCE?
Relaxxx
источник
20
Практика показывает, что большинство ответов о производительности, которые вы увидите здесь или в других местах, в большинстве случаев неверны (4 неверных 1 правильный). Мой совет - всегда делайте профилирование и проверяйте его самостоятельно, вы избежите «преждевременной оптимизации» и узнаете новое.
Marek R
25
@MarekR - это не просто производительность, это возможные побочные эффекты в OtherCunctionCall ...
relaxxx
3
@David, ссылаясь на другие сайты, часто бывает полезно указать, что кросс-постинг не одобряется
gnat
7
Если удобочитаемость - ваша главная задача, не вызывайте функции с побочными эффектами внутри условного if
Morgen
3
Потенциальные близкие избиратели: прочтите вопрос еще раз. Часть (1) не основана на мнении, тогда как часть (2) может легко перестать быть основанной на мнении, если отредактировать, удалив ссылку на любую предполагаемую «лучшую практику», как я собираюсь сделать.
duplode

Ответы:

119

Одно естественное решение могло бы выглядеть так:

bool b1 = SomeCondition();
bool b2 = b1 || SomeOtherCondition();
bool b3 = b2 || SomeThirdCondition();
// any other condition
bool bn = bn_1 || SomeFinalCondition();

if (bn)
{
  // do stuff
}

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


Это было моим первоначальным решением: вот хороший образец вызовов методов и тел цикла for:

if (!SomeComplicatedFunctionCall())
   return; // or continue

if (!SomeOtherComplicatedFunctionCall())
   return; // or continue

// do stuff

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

Хория Коман
источник
4
@relaxxx: Я понимаю, но "больше вещей, которые нужно сделать после if" также является признаком того, что ваша функция или метод слишком велики и должны быть разделены на более мелкие. Это не всегда лучший способ, но очень часто!
mike3996
2
это нарушает принцип белого списка
JoulinRouge
13
@JoulinRouge: Интересно, я никогда не слышал об этом принципе. Я лично предпочитаю этот подход "короткого замыкания" из-за преимуществ для читабельности: он уменьшает отступы и исключает вероятность того, что что-то произойдет после блока с отступом.
Matthieu M.
2
Это более читабельно? Назовите b2правильно, и у вас получится someConditionAndSomeotherConditionIsTrue, не супер-значимый. Кроме того, я должен хранить кучу переменных в моем мысленном стеке во время этого упражнения (и т. Д., Пока я не перестану работать в этой области). Я бы выбрал SJuan76решение номер 2 или просто поместил все это в функцию.
Натан Купер
2
Я не читал все комментарии, но после быстрого поиска не обнаружил большого преимущества первого фрагмента кода, а именно отладки. Размещение материала непосредственно в операторе if вместо того, чтобы заранее назначать его переменной, а затем вместо этого использовать переменную, делает отладку более сложной, чем это должно быть. Использование переменных также позволяет семантически группировать значения, что увеличивает удобочитаемость.
rbaleksandar
30

Я стараюсь разбивать условия на несколько строк, например:

if( SomeComplicatedFunctionCall()
 || OtherComplicatedFunctionCall()
  ) {

Даже при работе с несколькими операторами (&&) вам просто нужно увеличить отступ в каждой паре скобок. SCE по-прежнему срабатывает - нет необходимости использовать переменные. Написание кода таким образом сделало его более читаемым уже много лет. Более сложный пример:

if( one()
 ||( two()> 1337
  &&( three()== 'foo'
   || four()
    )
   )
 || five()!= 3.1415
  ) {
АмигоДжек
источник
28

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

bool b = SomeComplicatedFunctionCall() || OtherComplicatedFunctionCall();
if (b && some_other_expression) { ... }

Если у вас есть компилятор с поддержкой C ++ 11, вы можете использовать лямбда-выражения для объединения выражений в функции, как показано выше:

auto e = []()
{
    return SomeComplicatedFunctionCall() || OtherComplicatedFunctionCall();
};

if (e() && some_other_expression) { ... }
Какой-то чувак-программист
источник
21

1) Да у вас больше нет SCE. В противном случае у вас было бы это

bool b1 = SomeComplicatedFunctionCall();
bool b2 = OtherComplicatedFunctionCall();

работает так или иначе в зависимости от того, будет ли ifзаявление позже. Слишком сложно.

2) Это основано на мнении, но для достаточно сложных выражений вы можете:

if (SomeComplicatedFunctionCall()
    || OtherComplicatedFunctionCall()) {

Если это слишком сложно, очевидным решением будет создание функции, которая оценивает выражение и вызывает его.

SJuan76
источник
21

Вы также можете использовать:

bool b = someComplicatedStuff();
b = b || otherComplicatedStuff(); // it has to be: b = b || ...;  b |= ...; is bitwise OR and SCE is not working then 

и SCE будет работать.

Но это не намного читабельнее, чем например:

if (
    someComplicatedStuff()
    ||
    otherComplicatedStuff()
   )
КИИВ
источник
3
Мне не нравится комбинировать логические значения с побитовым оператором, мне это кажется не очень типичным. Обычно я использую то, что кажется наиболее читаемым, если только я не работаю на очень низком уровне и не считаю количество циклов процессора.
Ant
3
Я использовал специально, b = b || otherComplicatedStuff();и @SargeBorsch внес изменения, чтобы удалить SCE. Спасибо, что заметили меня об этом изменении @Ant.
КИИВ
14

1) Неужели я каждый раз теряю SCE? Может ли компилятор "оптимизировать его" и по-прежнему предоставлять SCE?

Я не думаю, что такая оптимизация допустима; особенно OtherComplicatedFunctionCall()могут иметь некоторые побочные эффекты.

2) Как лучше всего поступить в такой ситуации? Разве это единственная возможность (когда я хочу SCE) иметь все, что мне нужно, непосредственно внутри, если и «просто отформатировать его, чтобы он был как можно более читаемым»?

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

bool getSomeResult() {
    return SomeComplicatedFunctionCall() || OtherComplicatedFunctionCall();
}

...

if (getSomeResult())
{
    //do stuff
}

И поскольку мы реализуем getSomeResult()на основе SomeComplicatedFunctionCall()и OtherComplicatedFunctionCall(), мы можем рекурсивно декомпозировать их, если они все еще сложны.

Songyuanyao
источник
2
Мне это нравится, потому что вы можете получить некоторую читабельность, присвоив функции-оболочке описательное имя (хотя, вероятно, не getSomeResult), слишком много других ответов на самом деле не добавляют ничего ценного
aw04
9

1) Неужели я каждый раз теряю SCE? Может ли компилятор "оптимизировать" какой-то сценарий и по-прежнему предоставлять SCE?

Нет, нет, но это применяется по-другому:

if (SomeComplicatedFunctionCall() || OtherComplicatedFunctionCall())
{
    // do stuff
}

Здесь компилятор даже не запустится, OtherComplicatedFunctionCall()если SomeComplicatedFunctionCall()вернет true.

bool b1 = SomeComplicatedFunctionCall();
bool b2 = OtherComplicatedFunctionCall();

if (b1 || b2)
{
    //do stuff
}

Здесь будут работать обе функции , потому что они должны быть сохранены в b1и b2. b1 == trueТогда Ff b2не будет оцениваться (SCE). Но OtherComplicatedFunctionCall()уже пробег.

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

2) Как лучше всего поступить в такой ситуации? Это единственная возможность (когда я хочу, чтобы SCE) иметь все, что мне нужно, непосредственно внутри, если и «просто отформатировать его, чтобы он был как можно более читаемым»?

Это зависит от. Если вам нужно OtherComplicatedFunctionCall() запустить из-за побочных эффектов или снижение производительности функции минимально, вам следует использовать второй подход для удобства чтения. В противном случае придерживайтесь SCE в первом подходе.

Петух в шляпе
источник
8

Другая возможность короткого замыкания и условия в одном месте:

bool (* conditions [])()= {&a, &b, ...}; // list of conditions
bool conditionsHold = true;
for(int i= 0; i < sizeOf(conditions); i ++){
     if (!conditions[i]()){;
         conditionsHold = false;
         break;
     }
}
//conditionsHold is true if all conditions were met, otherwise false

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

левилим
источник
1
@Erbureth Нет, это не так. Элементы массива являются указателями на функции, они не выполняются, пока функции не будут вызваны в цикле.
Barmar
Спасибо, Бармар, но я внес правку, Эрбурет был прав, перед редактированием (я думал, что моя правка будет распространяться визуально более прямо).
levilime
4

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

if (somecomplicated_function() || // let me explain what this function does
    someother_function())         // this function does something else
...

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

/*---------------------------*/
/*! interpolates between values
* @param[in] X_axis : contains X-values
* @param[in] Y_axis : contains Y-values
* @param[in] value  : X-value, input to the interpolation process
* @return[out]      : the interpolated value
* @example          : interpolate([2,0],[3,2],2.4) -> 0.8
*/
int interpolate(std::vector<int>& X_axis, std::vector<int>& Y_axis, int value)

Очевидно, что форматирование ваших комментариев может зависеть от вашей среды разработки (Visual studio, JavaDoc в Eclipse, ...)

Что касается SCE, я предполагаю, что вы имеете в виду следующее:

bool b1;
b1 = somecomplicated_function(); // let me explain what this function does
bool b2 = false;
if (!b1) {                       // SCE : if first function call is already true,
                                 // no need to spend resources executing second function.
  b2 = someother_function();     // this function does something else
}

if (b1 || b2) {
...
}
Доминик
источник
-7

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

br0lly
источник
23
Принимая во внимание, что «ты через шесть месяцев» определенно «кто-то другой», а «ты завтра» иногда может быть. Я бы никогда не пожертвовал удобочитаемостью ради производительности, пока у меня не было убедительных доказательств того, что существует проблема с производительностью.
Мартин Боннер поддерживает Монику