Как можно назвать этот способ написания кода? - PullRequest
1 голос
/ 10 сентября 2009

Я рассматриваю довольно старый проект и уже во второй раз вижу такой код (C ++ - как псевдокод):

if( conditionA && conditionB ) {
   actionA();
   actionB();
} else {
   if( conditionA ) {
      actionA();
   }
   if( conditionB ) {
      actionB();
   }
}

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

if( conditionA ) {
   actionA();
}
if( conditionB ) {
   actionB();
}

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

Ответы [ 12 ]

13 голосов
/ 10 сентября 2009

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

6 голосов
/ 10 сентября 2009

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

4 голосов
/ 10 сентября 2009

Ребята? Посмотрите на эту часть: (условие A && условие B) По сути, если условие A ложно, оно не будет оценивать условие B.

Теперь, это был бы плохой стиль кодирования, но если бы условия A и условие B не просто оценивали данные, но если за этими условиями есть некоторый код, который изменяет данные, между этими двумя обозначениями может быть огромная разница!

если условие A ложно, то условие A оценивается дважды, а условие B оценивается только один раз. Если условие A истинно, а условие B ложно, то оба условия оцениваются дважды. Если оба условия выполняются, оба выполняются только один раз.

Во втором предложении оба условия выполняются только один раз ... Таким образом, эти методы эквивалентны, только если оба метода имеют значение true.

Чтобы усложнить ситуацию, если условие B ложно, то actionA может изменить что-то, что изменит эту проверку! Таким образом, ветвь else будет также выполнять actionB. Но если оба условия оцениваются как true, и actionA изменит оценку conditionB на false, оно все равно выполнит actionB.

Я склонен называть этот вид кода следующим образом: «Зачем делать вещи проще, когда вы можете делать это трудным путем?» и считают это шаблоном дизайна. На самом деле, это « Ипотечно-управляемая разработка », где код сделан более сложным, поэтому основной разработчик будет единственным, кто поймет его, в то время как другие разработчики просто запутаются и, надеюсь, сдадутся, чтобы полностью изменить дизайн. , В результате первоначальный разработчик должен оставаться только для того, чтобы поддерживать этот код, который называется «Безопасность работы», и, таким образом, иметь возможность платить по закладной в течение многих, многих лет.


Интересно, почему что-то подобное было бы использовал, затем понял, что я использую аналогичную структуру в моем собственном коде. Похоже, но не то же самое:
if (A&&B){
  action1;
} elseif(A){
  action2;
} elseif(B){
  action3;
} else{action4}

В этом случае каждое действие будет отображать другое сообщение. Сообщение, которое не может быть сгенерировано простым объединением двух строк. Скажем, это часть игры, и А проверяет, достаточно ли у вас энергии, а В проверяет, достаточно ли у вас массы. Из вас не хватает массы и энергии, вы больше ничего не можете построить, и необходимо отобразить критическое предупреждение. Если у вас есть только энергия, достаточно предупреждения о том, что вам нужно найти больше массы. Только строители должны перезаряжаться. И с обоими вы можете продолжать строить. Четыре разных действия, таким образом, это странная конструкция. Тем не менее, образец в Q показывает что-то совершенно другое. По сути, вы получите одно сообщение о том, что у вас нет массы, а другое - о том, что у вас нет энергии. Эти два сообщения не объединены в одно сообщение.

Теперь, в примере, если условие A будет определять уровни энергии, а условие B будет определять уровни массы, тогда оба решения будут работать нормально. Теперь, если actionA скажет вашим строителям сбросить их массу и начать перезаряжаться, вы внезапно снова наберете немного массы в своей игре. Но если бы условие B указывало на то, что у вас кончились массы, это больше не будет правдой! Просто потому, что actionA снова выпустил массу. если actionB будет командой, чтобы сказать строителям начать сбор массы, как только они смогут, тогда первое решение даст всем строителям эту команду, и они начнут собирать массу сначала, тогда они продолжат свои другие действия. Во втором решении такая команда не будет предоставлена. Строители перезаряжаются снова и начинают использовать только что выпущенную массу. Если эта проверка выполняется каждые 5 минут, то эти строители, например, перезарядите в течение одной минуты, чтобы бездействовать еще 4 минуты, потому что они исчерпали массу. В первом решении они сразу начнут собирать массу.

Да, это глупый пример. Снова играл в Supreme Commander. :-) Просто хотел придумать возможный сценарий, но он мог бы быть значительно улучшен! ...

3 голосов
/ 10 сентября 2009

Это очень близко к шаблону проектирования "fizzbuzz":

if( fizz && buzz ) {
   printFizz();
   printBuzz();
} else {
   if( fizz ) {
      printFizz();
   }
   else if( buzz ) {
      printBuzz();
   }
   else {
      printValue();
   }
}

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

3 голосов
/ 10 сентября 2009

Это код, написанный кем-то, кто не знает, как использовать Карта Карно .

2 голосов
/ 10 сентября 2009

неэффективный код?
Также можно назвать Оплачено за линию

2 голосов
/ 10 сентября 2009

Это «избыточный» код, и да, это плохо. Если какое-либо предварительное условие должно быть добавлено к вызовам actionA (при условии, что предварительное условие не может быть введено в само actionA), теперь нам нужно изменить код в 2 местах и, следовательно, рискнуть пропустить одно из них.

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

2 голосов
/ 10 сентября 2009

Я бы тоже назвал это плохим кодом.

Нет лучшего способа сделать отступ, но есть одно золотое правило: выберите один и придерживайтесь его.

1 голос
/ 10 сентября 2009

Как сказал pgast в комментарии, в этом коде нет ничего плохого, если actionA воздействует на условие B (обратите внимание, что это не условие с побочным эффектом, а действие с побочным эффектом (что вы ожидаете))

1 голос
/ 10 сентября 2009

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

  1. Код был изначально другим, подвергся рефакторингу, но тот, кто сделал это, наблюдал, что это избыточно.
  2. Кто бы это ни делал, он не понимал булеву логику.

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

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