Дополнительная строка в блоке против дополнительного параметра в чистом коде

33

контекст

В Чистом коде , на странице 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;
    }

Вопрос

Есть ли (не) преимущества, которых я не вижу, которые делают одно из них предпочтительным в целом? Или есть такие преимущества в определенных ситуациях; в таком случае, что я должен искать при принятии решения?

Р. Шмитц
источник
58
Что не так с flurps.Add(CreateFlurp(badaBoom));?
Мастер
47
Нет, это всего лишь одно утверждение. Это просто тривиально вложенное выражение (один вложенный уровень). И если простое f(g(x))против вашего руководства по стилю, ну, я не могу исправить ваше руководство по стилю. Я имею в виду, ты тоже не разбиваешься sqrt(x*x + y*y)на четыре строки? И это три (!) Вложенных подвыражения на двух (!) Внутренних уровнях вложенности (задыхаясь!). Ваша цель должна быть удобочитаемость , а не операторы одного оператора. Если вы хотите позже, у меня есть идеальный язык для вас: Ассемблер.
Мастер
6
@cmaster Даже в сборке x86 строго нет операторов с одним оператором. Режимы адресации памяти включают в себя множество сложных операций и могут использоваться для арифметики - на самом деле, вы можете сделать компьютер, полный Тьюринга, используя только movинструкции x86 и одну jmp toStartв конце. Кто-то на самом деле сделал компилятор, который делает именно это: D
Luaan
5
@Luaan Не говоря уже о печально известной rlwimiинструкции на КПП. (Это означает «Поворот левой вставки слова немедленной маски».) Эта команда приняла не менее пяти операндов (два регистра и три непосредственных значения) и выполнила следующие операции: содержимое одного регистра было повернуто с помощью немедленного сдвига, маска была созданный с одним прогоном из 1 бита, который контролировался двумя другими непосредственными операндами, и биты, которые соответствовали 1 биту в этой маске в другом операнде регистра, были заменены соответствующими битами повернутого регистра. Очень крутая инструкция :-)
cmaster
7
@ R.Schmitz «Я занимаюсь программированием общего назначения» - на самом деле нет, вы не программируете для конкретной цели (я не знаю, с какой целью, но я предполагаю, что вы делаете ;-). Существуют буквально тысячи целей для программирования, и оптимальные стили кодирования для них различаются - поэтому то, что подходит вам, может не подходить для других, и наоборот: часто совет здесь абсолютный (« всегда делайте X; Y плохой и т. д. игнорирование того факта, что в некоторых доменах придерживаться этого нецелесообразно. Вот почему советы в книгах, таких как Чистый код, всегда следует брать с
небольшим

Ответы:

104

Эти рекомендации - компас, а не карта. Они указывают вам в разумном направлении . Но они не могут сказать вам в абсолютном выражении, какое решение является «лучшим». В какой-то момент вам нужно прекратить движение в направлении, указанном вашим компасом, потому что вы прибыли в пункт назначения.

Чистый код поощряет вас делить код на очень маленькие, очевидные блоки. Это в целом хорошее направление. Но если довести до крайности (как следует из буквального толкования цитируемого совета), вы поделите свой код на бесполезно маленькие кусочки. Ничто на самом деле ничего не делает, все только делегаты. По сути, это другой вид обфускации кода.

Ваша задача - балансировать «чем меньше, тем лучше» и «слишком мало - бесполезно». Спросите себя, какое решение проще. Для меня это, безусловно, первое решение, поскольку оно, очевидно, собирает список. Это хорошо понятная идиома. Этот код можно понять, не обращая внимания на еще одну функцию.

Если это возможно сделать лучше, отметьте, что «преобразование всех элементов из списка в другой список» является распространенным шаблоном, который часто можно абстрагировать с помощью функциональной map()операции. В C # я думаю, что это называется Select. Что-то вроде этого:

public List<Flurp> CreateFlurps(List<BadaBoom> badaBooms)
{
    return badaBooms.Select(BadaBoom => CreateFlurp(badaBoom)).ToList();
}
Амон
источник
7
Код по-прежнему неверен, и он бессмысленно изобретает велосипед. Зачем звонить, CreateFlurps(someList)когда BCL уже предоставляет someList.ConvertAll(CreateFlurp)?
Бен Фойгт
44
@BenVoigt Это вопрос уровня дизайна. Меня не интересует точный синтаксис, тем более что у доски нет компилятора (и я последний раз писал C # в '09). Моя точка зрения не в том, что «я показал лучший из возможных кодов», а «в стороне, это общий шаблон, который уже решен». Linq - это один из способов сделать это, Convert. Вы упомянули другой . Спасибо, что предложили эту альтернативу.
Амон
1
Ваш ответ разумный, но тот факт, что LINQ абстрагирует логику и сокращает утверждение до одной строки, в конце концов, кажется, противоречит вашему совету. Как примечание стороны, BadaBoom => CreateFlurp(badaBoom)является избыточным; Вы можете передать CreateFlurpкак функцию напрямую ( Select(CreateFlurp)). (Насколько я знаю, так было всегда.)
jpmc26
2
Обратите внимание, что это полностью устраняет необходимость в методе. Название CreateFlurpsна самом деле вводит в заблуждение и труднее понять, чем просто видеть badaBooms.Select(CreateFlurp). Последнее полностью декларативно - нет проблем с разложением и, следовательно, нет необходимости в методе вообще.
Карл Лет
1
@ R.Schmitz Это не сложно понять, но это менее легко понять, чем badaBooms.Select(CreateFlurp). Вы создаете метод так, что его имя (высокий уровень) заменяет его реализацию (низкий уровень). В этом случае они находятся на одном уровне, поэтому, чтобы точно узнать, что происходит, мне нужно просто посмотреть на метод (вместо того, чтобы увидеть его встроенным). CreateFlurps(badaBooms)может держать сюрпризы, но badaBooms.Select(CreateFlurp)не может. Это также вводит в заблуждение, потому что он ошибочно просит Listвместо IEnumerable.
Карл Лет
61

Идеальное число аргументов для функции равно нулю (niladic)

Нет! Идеальное количество аргументов для функции - один. Если оно равно нулю, то вы гарантируете, что функция должна иметь доступ к внешней информации, чтобы иметь возможность выполнить действие. "Дядя" Боб понял это очень неправильно.

Что касается вашего кода, ваш первый пример имеет только две строки в блоке, потому что вы создаете локальную переменную в первой строке. Удалите это назначение, и вы соблюдаете следующие правила чистого кода:

public List<Flurp> CreateFlurps(List<BadaBoom> badaBooms)
{
    List<Flurp> flurps = new List<Flurp>();
    foreach (BadaBoom badaBoom in badaBooms)
    {
        flurps.Add(CreateFlurp(badaBoom));
    }
    return flurps;
}

Но это очень длинный код (C #). Просто сделайте это как:

IEnumerable<Flurp> CreateFlurps(IEnumerable<BadaBoom> badaBooms) =>
    from badaBoom in babaBooms select CreateFlurp(badaBoom);
Дэвид Арно
источник
14
Функция с нулевыми аргументами предназначена для того, чтобы подразумевать, что объект инкапсулирует требуемые данные, а не то, что вещи существуют в глобальном состоянии вне объекта.
Ryathal
19
@Ryathal, две точки: (1) если вы говорите о методах, то для большинства (всех?) ОО-языков этот объект выводится (или явно указывается в случае Python) в качестве первого параметра. В Java, C # и т. Д. Все методы являются функциями с хотя бы одним параметром. Компилятор просто скрывает эту деталь от вас. (2) Я никогда не упоминал «глобальный». Например, состояние объекта является внешним по отношению к методу.
Дэвид Арно,
17
Я уверен, что когда дядя Боб написал «ноль», он имел в виду «ноль (не считая этого)».
Док Браун
26
@DocBrown, вероятно, потому что он большой поклонник смешивания состояния и функциональности в объектах, поэтому под «функцией» он, вероятно, относится конкретно к методам. И я до сих пор не согласен с ним. Гораздо лучше дать методу только то, что ему нужно, а не оставлять его в предмете, чтобы получить то, что он хочет (т. Е. Это классический «говори, не спрашивай» в действии).
Дэвид Арно,
8
@AlessandroTeruzzi, идеал - это один параметр. Ноль слишком мало. Вот почему, например, функциональные языки принимают один как число параметров для целей каррирования (фактически в некоторых функциональных языках все функции имеют ровно один параметр: не больше, не меньше). Каррирование с нулевыми параметрами было бы бессмысленным. Заявление о том, что «идеал - это как можно меньше, эрго ноль - лучший», является примером сокращения до абсурда .
Дэвид Арно
19

Совет «Чистый код» совершенно неверен.

Используйте две или более строки в вашем цикле. Скрывать те же две строки в функции имеет смысл, когда они представляют собой случайную математику, которая нуждается в описании, но ничего не делает, когда строки уже являются описательными. «Создать» и «Добавить»

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

public List<Flurp> CreateFlurps(List<BadaBoom> badaBooms)
    {
        List<Flurp> flurps = new List<Flurp>();
        foreach (BadaBoom badaBoom in badaBooms)
        {
            flurps.Add(badaBoom .CreateFlurp());
            //or
            badaBoom.AddToListAsFlurp(flurps);
            //or
            flurps.Add(new Flurp(badaBoom));
            //or
            //make flurps a member of the class
            //use linq.Select()
            //etc
        }
        return flurps;
    }

или

foreach(var flurp in ConvertToFlurps(badaBooms))...

Как отмечали другие, совет о том, что лучшая функция - это функция без аргументов, в лучшем случае искажается до ООП, а в худшем - просто плохой совет.

Ewan
источник
Может быть, вы хотите отредактировать этот ответ, чтобы сделать его более понятным? Мой вопрос состоял в том, перевешивает ли одна вещь другую под Чистым Кодексом. Вы говорите, что все неправильно, и затем продолжаете описывать один из вариантов, которые я дал. На данный момент этот ответ выглядит так, будто вы следуете программе анти-Чистого кодекса, а не пытаетесь ответить на вопрос.
Р. Шмитц
извините, я истолковал ваш вопрос как предположение, что первый был «нормальным» способом, но вас подталкивали ко второму. Я вообще не против чистого кода, но эта цитата явно неверна
Ewan
19
@ R.Schmitz Я сам прочитал «Чистый код» и следую большей части того, что говорится в этой книге. Однако, что касается идеального размера функции, являющейся в значительной степени одним выражением, это просто неправильно. Единственный эффект заключается в том, что он превращает код спагетти в код риса. Читатель теряется во множестве тривиальных функций, которые дают осмысленный смысл только тогда, когда видятся вместе. Люди имеют ограниченную емкость оперативной памяти, и вы можете перегружать ее с помощью операторов или функций. Вы должны найти баланс между ними, если хотите быть читабельными. Избегайте крайностей!
Мастер
@cmaster Ответ был только первые 2 абзаца, когда я написал этот комментарий. Теперь это лучший ответ.
Р. Шмитц
7
честно говоря, я предпочел мой короткий ответ. В большинстве этих ответов слишком много дипломатических разговоров. Приведенный совет совершенно неверен, не нужно спекулировать на том, «что он на самом деле означает», или пытаться найти хорошую интерпретацию.
Эван
15

Второе, безусловно, хуже, так как CreateFlurpInListпринимает список и изменяет этот список, делая функцию не чистой и трудной для рассуждения. Ничто в названии метода не предполагает, что метод только добавляет в список.

И я предлагаю третий, лучший, вариант:

public List<Flurp> CreateFlurps(List<BadaBoom> badaBooms)
{
    return badaBooms.Select(CreateFlurp).ToList();
}

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

Euphoric
источник
Я бы не стал так сильно жаловаться на этот метод, «не будучи чистым и более сложным для рассуждения» (хотя и верным), а на то, что он является совершенно ненужным методом для обработки особого случая. Что если я захочу создать отдельную Flurp, Flurp, добавленную в массив, в словарь, Flurp, который затем будет найден в словаре, и соответствующая Flurp удалена и т. Д.? С тем же аргументом для кода Flurp также потребуются все эти методы.
gnasher729
10

Версия с одним аргументом лучше, но не в первую очередь из-за количества аргументов.

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

Если вы предоставите мне с CreateFlurp(BadaBoom), я могу использовать это с любым типом контейнера для сбора: Простой Flurp[], List<Flurp>, LinkedList<Flurp>, Dictionary<Key, Flurp>, и так далее. Но CreateFlurpInList(BadaBoom, List<Flurp>)завтра я вернусь к вам с просьбой, CreateFlurpInBindingList(BadaBoom, BindingList<Flurp>)чтобы моя модель представления смогла получить уведомление об изменении списка. Тьфу!

Как дополнительное преимущество, более простая подпись, скорее всего, будет соответствовать существующим API. Вы говорите, что у вас есть повторяющаяся проблема

довольно часто я создаю список из другого списка

Это просто вопрос использования доступных инструментов. Самый короткий, самый эффективный и лучший вариант:

var Flurps = badaBooms.ConvertAll(CreateFlurp);

Это не только меньше кода для написания и тестирования, но и более быстрый, потому что List<T>.ConvertAll()он достаточно умен, чтобы знать, что результат будет иметь то же количество элементов, что и вход, и предварительно распределить список результатов до правильного размера. Пока ваш код (обе версии) требовал расширения списка.

Бен Фойгт
источник
Не используйте List.ConvertAll. Вызывается идиоматический способ отображения множества объектов на различные объекты в C # Select. Единственная причина, ConvertAllдоступная даже здесь, заключается в том, что OP ошибочно запрашивает Listв методе - это должно быть IEnumerable.
Карл Лет
6

Помните об общей цели: сделать код легким для чтения и сопровождения.

Часто будет возможно сгруппировать несколько строк в одну значимую функцию. Сделайте так в этих случаях. Иногда вам нужно пересмотреть свой общий подход.

Например, в вашем случае замена всей реализации на var

flups = badaBooms.Select(bb => new Flurp(bb));

может быть возможность. Или вы могли бы сделать что-то вроде

flups.Add(new Flurp(badaBoom))

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

Ваш второй пример (на мой взгляд) значительно сложнее для понимания, чем первый. Дело не только в том, что у вас есть второй параметр, но и в том, что параметр модифицируется функцией. Посмотрите, что Чистый код говорит об этом. (У меня сейчас нет книги под рукой, но я уверен, что в основном это «не делай этого, если сможешь этого избежать»).

doubleYou
источник