Это небольшая проблема, но каждый раз, когда мне приходится кодировать что-то вроде этого, повторение беспокоит меня, но я не уверен, что какое-либо из решений не хуже.
if(FileExists(file))
{
contents = OpenFile(file); // <-- prevents inclusion in if
if(SomeTest(contents))
{
DoSomething(contents);
}
else
{
DefaultAction();
}
}
else
{
DefaultAction();
}
- Есть ли название для такой логики?
- Я тоже чересчур ОКР?
Я открыт для злых предложений по коду, хотя бы ради любопытства ...
coding-style
conditions
Benjol
источник
источник
DefaultAction
звонка нарушают принцип СУХОГОmake sure I have valid data for DoSomething(), and then DoSomething() with it. Otherwise, take DefaultAction()
. Мельчайшие подробности о том, что у вас есть данные для DoSomething (), находятся на более низком уровне абстракции и, следовательно, должны быть в другой функции. Эта функция будет иметь имя на более высоком уровне абстракции, а ее реализация будет на низком уровне. Хорошие ответы ниже решают эту проблему.Ответы:
Распакуйте его в отдельную функцию (метод) и используйте
return
оператор:Или, может быть, лучше разделить получение содержимого и его обработку:
Upd:
Почему бы не исключения, почему
OpenFile
не выдает исключения IO:я думаю, что это действительно общий вопрос, а не вопрос о файловом IO. Такие имена, как
FileExists
,OpenFile
могут вводить в заблуждение, но если заменить их наFoo
иBar
т. Д., То будет понятнее, что ониDefaultAction
могут вызываться так же часто, как иDoSomething
исключительный случай. Петер Тёрёк написал об этом в конце своего ответаПочему существует троичный условный оператор во втором варианте:
если бы был тег [C ++], я написал бы
if
заявление с объявлением ofcontents
в его условной части:Но для других (C-подобных) языков
if(contents) ...; else ...;
это точно так же, как выражение выражения с тернарным условным оператором, но длиннее. Поскольку основной частью кода былаget_contents
функция, я просто использовал более короткую версию (а также опущенныйcontents
тип). Во всяком случае, это за пределами этого вопроса.источник
Если язык программирования, который вы используете (0), выполняет короткое замыкание двоичных сравнений (т.е. если не вызывает,
SomeTest
еслиFileExists
возвращает false) и (1) присваивание возвращает значение (результатOpenFile
присваивается,contents
а затем это значение передается в качестве аргумента toSomeTest
), вы можете использовать что-то вроде следующего, но вам все равно советуют комментировать код, отмечая, что сингл=
является преднамеренным.В зависимости от того, насколько сложным является if, может быть лучше иметь переменную flag (которая разделяет тестирование условий успеха / неудачи с кодом, который обрабатывает ошибку
DefaultAction
в этом случае)источник
if
мой взгляд, довольно грубо вкладывать так много кода в утверждение.Более серьезным, чем повторение вызова DefaultAction, является сам стиль, потому что код написан не ортогонально (см. Этот ответ для веских причин написания ортогонально).
Чтобы показать, почему неортогональный код плох, рассмотрим оригинальный пример, когда вводится новое требование, чтобы мы не открывали файл, если он хранится на сетевом диске. Ну, тогда мы могли бы просто обновить код следующим образом:
Но также возникает требование, чтобы мы не открывали большие файлы более 2 ГБ. Ну, мы просто обновим снова:
Должно быть очень ясно, что такой стиль кода будет огромной проблемой обслуживания.
Среди ответов, которые написаны правильно ортогонально, есть второй пример Abyx и ответ Яна Худека , поэтому я не буду повторять это, просто укажу, что добавление двух требований в эти ответы будет просто
(или
goto notexists;
вместоreturn null;
), не влияя ни на какой другой код, кроме этих добавленных строк . Например, ортогональный.При тестировании общим правилом должно быть тестирование исключений, а не нормальный случай .
источник
Очевидно:
Вы сказали, что вы открыты даже для злых решений, так что, используя злые счета Гото, нет?
Фактически, в зависимости от контекста, это решение может быть менее злым, чем зло, совершающее действие дважды, или злые дополнительные переменные. Я обернул его в функцию, потому что он точно не будет в середине длинной функции (не в последнюю очередь из-за возврата в середине). Но чем долго функция не в порядке, точка.
Когда у вас есть исключения, их будет легче читать, особенно если вы можете использовать OpenFile и DoSomething, просто генерирующие исключение, если условия не выполняются, поэтому вам вообще не нужны явные проверки. С другой стороны, в C ++, Java и C # создание исключения является медленной операцией, поэтому с точки зрения производительности goto все еще предпочтительнее.
Обратите внимание на «зло»: C ++ FAQ 6.15 определяет «зло» как:
И это относится к
goto
этому контексту. Структурированные структуры управления потоком лучше в большинстве случаев, но когда вы попадаете в ситуацию, когда они накапливают слишком много своих зол, например, присваивание по условию, вложение более чем на 3 уровня, дублирование кода или длительные условия,goto
может просто закончиться быть менее злым.источник
for(;;) { if(!FileExists(file)) break; contents = OpenFile(file); if(!SomeTest(contents)) break; DoSomething(contents); return; } /* broken out */ DefaultAction();
goto
, потому что вы злоупотребляетеbreak
таким образом, что никто не ожидает, что оно будет злоупотреблено, поэтому у людей, читающих код, будет больше проблем с пониманием того, куда их ведет разрыв, чем с goto, который говорит об этом явно. Кроме того, вы используете бесконечный цикл, который будет запускаться только один раз, что будет довольно странно. К сожалению,do { ... } while(0)
он не совсем читабелен, потому что вы видите, что это всего лишь забавный блок, когда вы добираетесь до конца, а C не поддерживает разрыв с другими блоками (в отличие от perl)....
источник
SomeTest
может иметь ту же семантику, что и существование файла, еслиSomeTest
проверяет тип файла, например, проверяет, что .gif действительно GIF-файл.contents && f(contents)
. Две функции, чтобы спасти еще одну ?!Одна возможность:
Конечно, это делает код немного более сложным по-другому. Так что это во многом вопрос стиля.
Другой подход будет использовать исключения, например:
Это выглядит проще, однако это применимо только если
DefaultAction()
подходит каждомуSomeTest()
явно являются ошибочным условием, поэтому целесообразно создать для него исключение.источник
(function () { ... })()
в Javascript,{ flag = false; ... }
в C-подобном и т. д.Это на более высоком уровне абстракции:
И это заполняет детали.
источник
Некоторые люди находят такой подход немного экстремальным, но он также очень чистый. Позвольте мне проиллюстрировать на Python:
Когда он говорит, что функции должны делать одно, он имеет в виду одно .
processFile()
выбирает действие, основанное на результате теста, и это все, что он делает.fileMeetsTest()
сочетает в себе все условия теста, и это все, что он делает.contentsTest()
передает первую строкуfirstLineTest()
, и это все, что он делает.Вроде бы много функций, но читается практически как прямой английский:
Конечно, это немного словесно, но учтите, что если сопровождающий не заботится о деталях, он может прекратить чтение только после четырех строк кода
processFile()
, и он все равно будет хорошо знать, что делает функция.источник
Что касается того, что это называется, он может легко превратиться в антишаблон со стрелками по мере того, как ваш код расширяется для удовлетворения дополнительных требований (как показано в ответе на https://softwareengineering.stackexchange.com/a/122625/33922 ) и затем попадает в ловушку огромных участков кода с вложенными условными выражениями, которые напоминают стрелку.
Смотрите ссылки, такие как;
http://codinghorror.com/blog/2006/01/flattening-arrow-code.html
http://lostechies.com/chrismissal/2009/05/27/anti-patterns-and-worst-practices-the-arrowhead-anti-pattern/
Об этом и других анти-паттернах можно найти еще много в Google.
Вот несколько замечательных советов, которые Джефф дает в своем блоге по этому поводу;
Посмотрите некоторые комментарии в блоге Джеффа относительно предложений Стива Макконнелса о досрочном возвращении;
Я всегда подписывался на 1 вход / выход на теорию функций из-за того, что меня учили 15 лет назад или около того. Я чувствую, что это делает код намного проще для чтения и, как вы упоминаете, более понятным
источник
Это соответствует правилам СУХОЙ, no-goto и no-множественный возврат, является масштабируемым и читаемым на мой взгляд:
источник
if
s в другиеif
s. Кроме того, код для обработки неудачного case (DefaultAction()
) находится только в одном месте, и в целях отладки код не перемещается по вспомогательным функциям, а добавление точек останова в строки, вsuccess
которых изменяется переменная, может быстро показать, какие тесты пройдены (выше сработавших). точка останова) и какие из них не были проверены (ниже).success
вok_so_far
:)Я бы извлек его в отдельный метод, а затем:
что также позволяет
тогда, возможно, вы могли бы удалить
DefaultAction
вызовы и оставить выполнениеDefaultAction
для вызывающего:Мне также нравится подход Жанны Пиндар .
источник
Для этого конкретного случая ответ достаточно прост ...
Существует состояние гонки между
FileExists
иOpenFile
: что произойдет , если файл будет удален?Единственный разумный способ справиться с этим конкретным случаем - пропустить
FileExists
:Это аккуратно решает эту проблему и делает код чище.
В общем: попробуйте переосмыслить проблему и найти другое решение, которое полностью исключает проблему.
источник
Другая возможность, если вам не нравится видеть слишком много других, - это вообще отказаться от использования else и добавить дополнительный оператор return. Остальное является излишним, если вам не требуется более сложная логика, чтобы определить, существует ли более двух возможных действий.
Таким образом, ваш пример может стать:
Лично я не возражаю против использования предложения else , так как оно явно указывает, как должна работать логика, и, таким образом, улучшает читабельность вашего кода. Однако некоторые инструменты для улучшения кода предпочитают упрощать до одного оператора if, чтобы не допустить вложения логики.
источник
Случай, показанный в примере кода, обычно сводится к одному
if
выражению. Во многих системах функция открытия файла возвращает недопустимое значение, если файл еще не существует. Иногда это поведение по умолчанию; в других случаях он должен быть указан через аргумент. Это означает, чтоFileExists
тест можно отменить, что также может помочь в условиях гонки, возникающих в результате удаления файла между тестом на существование и открытием файла.Это напрямую не решает проблему смешивания на уровне абстракции, поскольку полностью обходит проблему множественных, несвязанных тестов, хотя отказ от теста на существование файла не является несовместимым с разделением уровней абстракции. Предполагая, что недействительные файловые дескрипторы эквивалентны «false», и файловые дескрипторы закрываются, когда они выходят из области видимости:
источник
Я согласен с frozenkoi, однако, для C # в любом случае, я думал, что это поможет следовать синтаксису методов TryParse.
источник
Ваш код ужасен, потому что вы делаете слишком много в одной функции. Вы хотите обработать файл или выполнить действие по умолчанию, поэтому начните с того, что:
Программисты на Perl и Ruby пишут
processFile(file) || defaultAction()
Теперь иди напиши ProcessFile:
источник
Конечно, вы можете пойти так далеко только в таких сценариях, но вот путь:
Вы можете хотеть дополнительные фильтры. Затем сделайте это:
Хотя это также может иметь смысл:
Который лучший? Это зависит от реальной проблемы, с которой вы сталкиваетесь.
Но вот что нужно отнять: вы можете многое сделать с композицией и полиморфизмом.
источник
Что не так с очевидным
Это кажется довольно стандартным для меня? Для такой большой процедуры, где должно случиться много мелочей, отказ любого из которых помешает последнему. Исключения делают его немного чище, если это вариант.
источник
Я понимаю, что это старый вопрос, но я заметил образец, который не был упомянут; в основном, установка переменной для последующего определения метода (ов), который вы хотели бы вызвать (за исключением if ... else ...).
Это просто еще один угол зрения, чтобы облегчить работу с кодом. Это также позволяет вам, когда вы захотите добавить другой метод для вызова или изменить соответствующий метод, который необходимо вызывать в определенных ситуациях.
Вместо того, чтобы заменить все упоминания о методе (и, возможно, пропустить некоторые сценарии), все они перечислены в конце блока if ... else ... и их легче читать и изменять. Я склонен использовать это, когда, например, несколько методов могут быть вызваны, но внутри вложенного if ... else ... метод может быть вызван в нескольких совпадениях.
Если вы установите переменную, которая определяет состояние, вы можете иметь много глубоко вложенных параметров и обновлять состояние, когда что-то должно быть (или не должно быть) выполнено.
Это можно использовать, как в примере, заданном в вопросе, где мы проверяем, произошло ли «DoSomething», и, если нет, выполните действие по умолчанию. Или у вас может быть состояние для каждого метода, который вы хотите вызвать, установить, когда это применимо, а затем вызвать соответствующий метод за пределами if ... else ...
В конце вложенных операторов if ... else ... вы проверяете состояние и действуете соответствующим образом. Это означает, что вам нужно только одно упоминание метода вместо всех мест, где он должен применяться.
источник
Чтобы уменьшить вложенный IF:
1 / досрочное возвращение;
2 / сложное выражение (с учетом короткого замыкания)
Итак, ваш пример может быть изменен следующим образом:
источник
Я видел много примеров с «return», которые я тоже использую, но иногда я хочу избежать создания новых функций и использовать вместо этого цикл:
Если вы хотите писать меньше строк или ненавидите бесконечные циклы, как я, вы можете изменить тип цикла на «do ... while (0)» и избежать последнего «разрыва».
источник
Как насчет этого решения:
Я сделал предположение, что OpenFile возвращает указатель, но это также может работать с возвращаемым типом значения, указав не возвращаемое значение по умолчанию (коды ошибок или что-то в этом роде).
Конечно, я не ожидаю каких-либо возможных действий с помощью метода SomeTest для указателя NULL (но вы никогда не узнаете), поэтому это также можно рассматривать как дополнительную проверку для указателя NULL для вызова SomeTest (содержимое).
источник
Понятно, что наиболее элегантным и лаконичным решением является использование макроса препроцессора.
Который позволяет вам написать красивый код следующим образом:
Может быть трудно полагаться на автоматическое форматирование, если вы часто используете эту технику, и некоторые IDE могут кричать вам немного о том, что, по ошибкам, ошибочно. И, как говорится, все является компромиссом, но я полагаю, что это неплохая цена, чтобы избежать зла повторяющегося кода.
источник
Поскольку вы спросили из любопытства, и ваш вопрос не помечен конкретным языком (даже если ясно, что вы имели в виду императивные языки), возможно, стоит добавить, что языки, поддерживающие отложенную оценку, допускают совершенно иной подход. В этих языках выражения оцениваются только при необходимости, поэтому вы можете определять «переменные» и использовать их только тогда, когда это имеет смысл. Например, в вымышленном языке с ленивыми
let
/in
структурами вы забыли о контроле потока и записи:источник