Я пытаюсь следовать рекомендациям дяди Боба по чистому коду и, в частности, держать методы короткими
Я не могу сократить эту логику, хотя:
if (checkCondition()) {addAlert(1);}
else if (checkCondition2()) {addAlert(2);}
else if (checkCondition3()) {addAlert(3);}
else if (checkCondition4()) {addAlert(4);}
Я не могу удалить elses и, таким образом, разделить все это на более мелкие биты, потому что «else» в «else if» помогает производительности - оценка этих условий стоит дорого, и, если я могу избежать оценки условий ниже, вызову одно из первых это правда, я хочу их избежать.
Даже с семантической точки зрения, оценка следующего условия, если выполнено предыдущее, не имеет смысла с точки зрения бизнеса.
редактировать: этот вопрос был определен как возможный дубликат элегантных способов обработки, если (если еще) еще .
Я считаю, что это другой вопрос (вы можете увидеть это также, сравнив ответы на эти вопросы).
- Мой вопрос проверяет, чтобы первое принятое условие закончилось быстро .
- Связанный вопрос пытается создать все условия, чтобы принять что-то. (лучше видно в этом ответе на этот вопрос: https://softwareengineering.stackexchange.com/a/122625/96955 )
clean-code
conditions
Ev0oD
источник
источник
Ответы:
В идеале я думаю, что вы должны извлечь свою логику для получения кода / номера оповещения в свой собственный метод. Таким образом, ваш существующий код сокращен до
и у вас есть GetConditionCode () инкапсулировать логику для проверки условий. Возможно также лучше использовать Enum, чем магическое число.
источник
addAlert
проверять наличие фиктивного состояния предупрежденияAlertCode.None
.Важным измерением является сложность кода, а не абсолютный размер. Предполагая, что различные условия на самом деле являются просто вызовами одной функции, точно так же, как действия не более сложны, чем то, что вы показали, я бы сказал, что с кодом все в порядке. Это уже так просто, как может быть.
Любая попытка дальнейшего «упрощения» действительно усложнит ситуацию.
Конечно, вы можете заменить
else
ключевое слово на,return
как предлагали другие, но это просто вопрос стиля, а не изменение сложности вообще.В стороне:
Мой общий совет заключается в том, чтобы никогда не становиться религиозным в отношении какого-либо правила для чистого кода: большинство советов по кодированию, которые вы видите в Интернете, хороши, если они применяются в подходящем контексте, но радикальное применение этого же совета везде может привести к попаданию в IOCCC . Хитрость заключается в том, чтобы всегда находить баланс, который позволяет людям легко рассуждать о вашем коде.
Используйте слишком большие методы, и вы облажались. Используйте слишком маленькие функции, и вы облажались. Избегайте троичных выражений, и вы облажались. Используйте троичные выражения везде, и вы облажались. Поймите, что есть места, которые вызывают однострочные функции, и места, которые вызывают 50-строчные функции (да, они существуют!). Поймите, что есть места, которые требуют
if()
операторов, и что есть места, которые требуют?:
оператора. Используйте полный арсенал, который есть в вашем распоряжении, и старайтесь всегда использовать наиболее подходящий инструмент, который вы можете найти. И помните, не будьте религиозны даже об этом совете.источник
else if
внутреннего сreturn
последующим простымif
(удалениеelse
) может сделать код более трудным для чтения . Когда код говоритelse if
, я сразу знаю, что код в следующем блоке будет выполняться только в том случае, если предыдущий не выполнял. Нет суеты, нет суеты. Если это равнина,if
то он может выполняться или не выполняться, независимо от того, был ли выполнен предыдущий. Теперь мне придется потратить некоторое количество умственных усилий, чтобы разобрать предыдущий блок, чтобы отметить, что он заканчивается наreturn
. Я бы лучше потратил эти умственные усилия на разбор бизнес-логики.else if
одна семантическая единица. (Это не обязательно отдельный модуль для компилятора, но это нормально.)...; return; } if (...
Не; не говоря уже о том, если он разбит на несколько строк. Это то, на что я действительно должен смотреть, чтобы увидеть, что он делает, вместо того, чтобы иметь возможность принять это непосредственно, просто увидев пару ключевых словelse if
.else if
конструкцию, тем более что ее цепочечная форма - такой хорошо известный шаблон. Тем не менее, код формыif(...) return ...;
также является широко известным шаблоном, поэтому я не буду полностью осуждать это. Однако я считаю это незначительной проблемой: логика потока управления одинакова в обоих случаях, и один более внимательный взгляд наif(...) { ...; return; }
лестницу скажет мне, что она действительно эквивалентнаelse if
лестнице. Я вижу структуру одного термина, я понимаю его значение, я понимаю, что он повторяется везде, и я знаю, что случилось.else if
и другоеreturn
. например,else if(...) { return alert();}
Это спорное ли это «лучше» , чем обычный if..else для любого конкретного случая. Но если вы хотите попробовать что-то еще, это обычный способ сделать это.
Поместите ваши условия в объекты и поместите эти объекты в список
Если требуется несколько действий при условии, вы можете сделать некоторую сумасшедшую рекурсию
Очевидно, да. Это работает, только если у вас есть образец для вашей логики. Если вы попытаетесь выполнить суперобобщенное рекурсивное условное действие, то настройка для объекта будет такой же сложной, как и оригинальная инструкция if. Вы будете изобретать свой новый язык / рамки.
Но ваш пример действительно есть шаблон
Распространенным вариантом использования этого шаблона будет проверка. Вместо :
становится
источник
if...else
лестницу в построенииConditions
списка. Чистый выигрыш отрицателен, так как построениеConditions
займет столько же кода, сколько и код OP, но добавленная косвенность приводит к затратам на читабельность. Я бы определенно предпочел чистую кодовую лестницу.conditions
устроено ... АРГ! Не аннотации-атрибуты! Господи, почему? О, мои глаза!Подумайте об использовании
return;
после выполнения одного условия, оно спасет вас от всехelse
ошибок. Вы могли бы даже иметь возможностьreturn addAlert(1)
напрямую, если этот метод имеет возвращаемое значение.источник
if
с ... Это может быть разумным предположением, а с другой стороны, это может и не произойти.Я видел такие конструкции, которые иногда считаются более чистыми:
Тройной с правильным интервалом также может быть аккуратной альтернативой:
Я думаю, вы могли бы также попытаться создать массив с парой, содержащей условие и функцию, и выполнять итерацию по ней до тех пор, пока не будет выполнено первое условие, которое, как я вижу, будет равно первому ответу Эвана.
источник
case
метками?В качестве варианта ответа @ Ewan вы можете создать цепочку (вместо «плоского списка») таких условий:
Таким образом, вы можете применять свои условия в определенном порядке, и инфраструктура (показан абстрактный класс) пропускает оставшиеся проверки после того, как первая была выполнена.
Именно здесь он превосходит подход «плоского списка», где необходимо реализовать «пропуск» в цикле, который применяет условия.
Вы просто настраиваете цепочку условий:
И начните оценку с простого вызова:
источник
Прежде всего, оригинальный код не страшен IMO. Это довольно понятно, и в этом нет ничего плохого.
Тогда, если вам это не нравится, опираясь на идею @ Ewan использовать список, но удаляя его несколько неестественный
foreach break
шаблон:Адаптируйте это по своему выбору, сделайте каждый элемент списка объектом, кортежем, чем угодно, и все хорошо.
РЕДАКТИРОВАТЬ: похоже, это не так ясно, как я думал, поэтому позвольте мне объяснить подробнее.
conditions
упорядоченный список какого-то рода;head
текущий исследуемый элемент - в начале это первый элемент списка, и при каждомnext()
вызове он становится следующим;check()
иalert()
являютсяcheckConditionX()
иaddAlert(X)
от ОП.источник
while not
вместоforeach break
.В этом вопросе отсутствует определенность. Если условия:
или если содержание в
addAlert
более сложное, то, возможно, лучшее решение, скажем, в c # будет:Кортежи не так красивы в c # <8, но выбраны для удобства.
Плюсы этого метода, даже если ни один из перечисленных выше вариантов не применим, заключается в том, что структура имеет статическую типизацию. Вы не можете случайно облажаться, скажем, пропустив
else
.источник
Лучший способ уменьшить цикломатическую сложность в тех случаях, когда у вас их много,
if->then statements
- это использовать словарь или список (в зависимости от языка) для хранения значения ключа (если значение оператора или какое-либо значение), а затем значения / результата функции.Например, вместо (C #):
Я могу просто
Если вы используете более современные языки, вы можете хранить больше логики, чем просто значения (c #). На самом деле это просто встроенные функции, но вы также можете просто указать на другие функции, если логика должна быть просто встроенной.
источник
Ваш код уже слишком короткий, но сама логика не должна изменяться. На первый взгляд кажется, что вы повторяете себя четырьмя вызовами
checkCondition()
, и только очевидно, что каждый из них отличается после тщательного перечитывания кода. Вы должны добавить правильное форматирование и имена функций, например:Ваш код должен быть удобочитаемым. Прочитав несколько книг дяди Боба, я считаю, что это послание, которое он постоянно пытается донести.
источник
Предполагая, что все функции реализованы в одном и том же компоненте, можно заставить функции сохранять некоторое состояние, чтобы избавиться от нескольких ветвей в потоке.
EG:
checkCondition1()
стал быevaluateCondition1()
, на котором он будет проверять, было ли выполнено предыдущее условие; если это так, то он кэширует какое-то значение для извлеченияgetConditionNumber()
.checkCondition2()
станетevaluateCondition2()
, на котором он будет проверять, были ли выполнены предыдущие условия. Если предыдущее условие не было выполнено, выполняется проверка сценария условия 2 с кэшированием значения, которое должно быть полученоgetConditionNumber()
. И так далее.РЕДАКТИРОВАТЬ:
Вот как должна быть реализована проверка на дорогостоящие условия, чтобы этот подход работал.
Поэтому, если у вас слишком много дорогостоящих проверок, которые нужно выполнить, и вещи в этом коде остаются закрытыми, этот подход помогает поддерживать его, позволяя при необходимости изменить порядок проверок.
Этот ответ просто дает некоторые альтернативные предложения из других ответов и, вероятно, будет не лучше исходного кода, если мы рассмотрим только 4 строки кода. Хотя это не страшный подход (и ни один из них не усложняет обслуживание, как говорили другие), учитывая сценарий, который я упомянул (слишком много проверок, только основная функция выставлена как общедоступная, все функции являются деталями реализации одного и того же класса).
источник
anyCondition() != false
.true
, OP не хочет, чтобы условие 3 оценивалось.anyCondition() != false
в функцияхevaluateConditionXX()
. Это возможно реализовать. Если подход с использованием внутреннего состояния нежелателен, я понимаю, но аргумент, что это не работает, недопустим.Более двух предложений «else» вынуждают читателя кода пройти всю цепочку, чтобы найти интересующую его. Используйте метод, такой как: void AlertUponCondition (Условие условия) {switch (условие) {case Condition.Con1: ... break; case Condition.Con2: ... break; и т. д.}} Где «Condition» - это правильное перечисление. При необходимости верните бул или значение. Назовите это так: AlertOnCondition (GetCondition ());
Это действительно не может быть проще, и это быстрее, чем цепочка if-else, если вы превысите несколько случаев.
источник
Я не могу говорить о вашей конкретной ситуации, потому что код не является конкретным, но ...
Подобный код часто является запахом для недостающей ОО модели. У вас действительно есть четыре типа вещей, каждый из которых связан со своим собственным типом оповещателя, но вместо того, чтобы распознавать эти сущности и создавать для каждого экземпляр класса, вы рассматриваете их как одну вещь и пытаетесь восполнить это позже, в то время, когда вы действительно нужно знать, с чем вы имеете дело, чтобы продолжить.
Полиморфизм мог бы подойти вам лучше.
С подозрением относитесь к коду с длинными методами, содержащими длинные или сложные конструкции if-then. Вам часто нужно дерево классов с некоторыми виртуальными методами.
источник