контекст
В Чистом коде , на странице 35, написано
Это подразумевает, что блоки внутри операторов if, операторов else, операторов while и т. Д. Должны быть длиной в одну строку. Вероятно, эта строка должна быть вызовом функции. Это не только уменьшает объем включаемой функции, но также добавляет документальное значение, поскольку функция, вызываемая внутри блока, может иметь приятное описательное имя.
Я полностью согласен, это имеет большой смысл.
Позже, на странице 40, говорится об аргументах функции
Идеальное число аргументов для функции равно нулю (niladic). Далее следует одно (монадическое), за которым следует два (диадическое). По возможности следует избегать трех аргументов (триадных). Более трех (полиадических) требует особого обоснования, и их не следует использовать в любом случае. Аргументы жесткие. Они берут много концептуальной силы.
Я полностью согласен, это имеет большой смысл.
вопрос
Однако, довольно часто я создаю список из другого списка, и мне придется жить с одним из двух зол.
Либо я использую две строки в блоке , одну для создания вещи, одну для добавления ее к результату:
public List<Flurp> CreateFlurps(List<BadaBoom> badaBooms)
{
List<Flurp> flurps = new List<Flurp>();
foreach (BadaBoom badaBoom in badaBooms)
{
Flurp flurp = CreateFlurp(badaBoom);
flurps.Add(flurp);
}
return flurps;
}
Или я добавляю аргумент в функцию для списка, в который будет добавлена вещь, что делает ее «на один аргумент хуже».
public List<Flurp> CreateFlurps(List<BadaBoom> badaBooms)
{
List<Flurp> flurps = new List<Flurp>();
foreach (BadaBoom badaBoom in badaBooms)
{
CreateFlurpInList(badaBoom, flurps);
}
return flurps;
}
Вопрос
Есть ли (не) преимущества, которых я не вижу, которые делают одно из них предпочтительным в целом? Или есть такие преимущества в определенных ситуациях; в таком случае, что я должен искать при принятии решения?
источник
flurps.Add(CreateFlurp(badaBoom));
?f(g(x))
против вашего руководства по стилю, ну, я не могу исправить ваше руководство по стилю. Я имею в виду, ты тоже не разбиваешьсяsqrt(x*x + y*y)
на четыре строки? И это три (!) Вложенных подвыражения на двух (!) Внутренних уровнях вложенности (задыхаясь!). Ваша цель должна быть удобочитаемость , а не операторы одного оператора. Если вы хотите позже, у меня есть идеальный язык для вас: Ассемблер.mov
инструкции x86 и однуjmp toStart
в конце. Кто-то на самом деле сделал компилятор, который делает именно это: Drlwimi
инструкции на КПП. (Это означает «Поворот левой вставки слова немедленной маски».) Эта команда приняла не менее пяти операндов (два регистра и три непосредственных значения) и выполнила следующие операции: содержимое одного регистра было повернуто с помощью немедленного сдвига, маска была созданный с одним прогоном из 1 бита, который контролировался двумя другими непосредственными операндами, и биты, которые соответствовали 1 биту в этой маске в другом операнде регистра, были заменены соответствующими битами повернутого регистра. Очень крутая инструкция :-)Ответы:
Эти рекомендации - компас, а не карта. Они указывают вам в разумном направлении . Но они не могут сказать вам в абсолютном выражении, какое решение является «лучшим». В какой-то момент вам нужно прекратить движение в направлении, указанном вашим компасом, потому что вы прибыли в пункт назначения.
Чистый код поощряет вас делить код на очень маленькие, очевидные блоки. Это в целом хорошее направление. Но если довести до крайности (как следует из буквального толкования цитируемого совета), вы поделите свой код на бесполезно маленькие кусочки. Ничто на самом деле ничего не делает, все только делегаты. По сути, это другой вид обфускации кода.
Ваша задача - балансировать «чем меньше, тем лучше» и «слишком мало - бесполезно». Спросите себя, какое решение проще. Для меня это, безусловно, первое решение, поскольку оно, очевидно, собирает список. Это хорошо понятная идиома. Этот код можно понять, не обращая внимания на еще одну функцию.
Если это возможно сделать лучше, отметьте, что «преобразование всех элементов из списка в другой список» является распространенным шаблоном, который часто можно абстрагировать с помощью функциональной
map()
операции. В C # я думаю, что это называетсяSelect
. Что-то вроде этого:источник
CreateFlurps(someList)
когда BCL уже предоставляетsomeList.ConvertAll(CreateFlurp)
?BadaBoom => CreateFlurp(badaBoom)
является избыточным; Вы можете передатьCreateFlurp
как функцию напрямую (Select(CreateFlurp)
). (Насколько я знаю, так было всегда.)CreateFlurps
на самом деле вводит в заблуждение и труднее понять, чем просто видетьbadaBooms.Select(CreateFlurp)
. Последнее полностью декларативно - нет проблем с разложением и, следовательно, нет необходимости в методе вообще.badaBooms.Select(CreateFlurp)
. Вы создаете метод так, что его имя (высокий уровень) заменяет его реализацию (низкий уровень). В этом случае они находятся на одном уровне, поэтому, чтобы точно узнать, что происходит, мне нужно просто посмотреть на метод (вместо того, чтобы увидеть его встроенным).CreateFlurps(badaBooms)
может держать сюрпризы, ноbadaBooms.Select(CreateFlurp)
не может. Это также вводит в заблуждение, потому что он ошибочно проситList
вместоIEnumerable
.Нет! Идеальное количество аргументов для функции - один. Если оно равно нулю, то вы гарантируете, что функция должна иметь доступ к внешней информации, чтобы иметь возможность выполнить действие. "Дядя" Боб понял это очень неправильно.
Что касается вашего кода, ваш первый пример имеет только две строки в блоке, потому что вы создаете локальную переменную в первой строке. Удалите это назначение, и вы соблюдаете следующие правила чистого кода:
Но это очень длинный код (C #). Просто сделайте это как:
источник
Совет «Чистый код» совершенно неверен.
Используйте две или более строки в вашем цикле. Скрывать те же две строки в функции имеет смысл, когда они представляют собой случайную математику, которая нуждается в описании, но ничего не делает, когда строки уже являются описательными. «Создать» и «Добавить»
Второй метод, о котором вы упоминаете, на самом деле не имеет никакого смысла, так как вы не обязаны добавлять второй аргумент, чтобы избежать двух строк.
или
Как отмечали другие, совет о том, что лучшая функция - это функция без аргументов, в лучшем случае искажается до ООП, а в худшем - просто плохой совет.
источник
Второе, безусловно, хуже, так как
CreateFlurpInList
принимает список и изменяет этот список, делая функцию не чистой и трудной для рассуждения. Ничто в названии метода не предполагает, что метод только добавляет в список.И я предлагаю третий, лучший, вариант:
И, черт возьми, вы можете сразу встроить этот метод, если есть только одно место, где он используется, так как однострочник сам по себе понятен, поэтому его не нужно инкапсулировать методом, чтобы придать ему смысл.
источник
Версия с одним аргументом лучше, но не в первую очередь из-за количества аргументов.
Самая важная причина, по которой он лучше, заключается в том, что он имеет более низкую связь , что делает его более полезным, более легким для рассуждения, более простым для тестирования и с меньшей вероятностью превратится в клоны с копированием + вставкой.
Если вы предоставите мне с
CreateFlurp(BadaBoom)
, я могу использовать это с любым типом контейнера для сбора: ПростойFlurp[]
,List<Flurp>
,LinkedList<Flurp>
,Dictionary<Key, Flurp>
, и так далее. НоCreateFlurpInList(BadaBoom, List<Flurp>)
завтра я вернусь к вам с просьбой,CreateFlurpInBindingList(BadaBoom, BindingList<Flurp>)
чтобы моя модель представления смогла получить уведомление об изменении списка. Тьфу!Как дополнительное преимущество, более простая подпись, скорее всего, будет соответствовать существующим API. Вы говорите, что у вас есть повторяющаяся проблема
Это просто вопрос использования доступных инструментов. Самый короткий, самый эффективный и лучший вариант:
Это не только меньше кода для написания и тестирования, но и более быстрый, потому что
List<T>.ConvertAll()
он достаточно умен, чтобы знать, что результат будет иметь то же количество элементов, что и вход, и предварительно распределить список результатов до правильного размера. Пока ваш код (обе версии) требовал расширения списка.источник
List.ConvertAll
. Вызывается идиоматический способ отображения множества объектов на различные объекты в C #Select
. Единственная причина,ConvertAll
доступная даже здесь, заключается в том, что OP ошибочно запрашиваетList
в методе - это должно бытьIEnumerable
.Помните об общей цели: сделать код легким для чтения и сопровождения.
Часто будет возможно сгруппировать несколько строк в одну значимую функцию. Сделайте так в этих случаях. Иногда вам нужно пересмотреть свой общий подход.
Например, в вашем случае замена всей реализации на var
может быть возможность. Или вы могли бы сделать что-то вроде
Иногда самое чистое и удобочитаемое решение просто не помещается в одну строку. Таким образом, у вас будет две строки. Не делайте код сложнее для понимания, просто для выполнения произвольного правила.
Ваш второй пример (на мой взгляд) значительно сложнее для понимания, чем первый. Дело не только в том, что у вас есть второй параметр, но и в том, что параметр модифицируется функцией. Посмотрите, что Чистый код говорит об этом. (У меня сейчас нет книги под рукой, но я уверен, что в основном это «не делай этого, если сможешь этого избежать»).
источник