Мой босс просит меня прекратить писать небольшие функции и делать все в одном цикле

209

Я прочитал книгу под названием « Чистый код » Роберта Мартина. В этой книге я видел много методов для очистки кода, таких как написание небольших функций, тщательный выбор имен и т. Д. Это, безусловно, самая интересная книга о чистом коде, которую я читал. Однако сегодня моему боссу не понравилось, как я написал код после прочтения этой книги.

Его аргументы были

  • Написание маленьких функций - это боль, потому что это заставляет вас переходить к каждой маленькой функции, чтобы посмотреть, что делает код.
  • Поместите все в основной большой цикл, даже если основной цикл содержит более 300 строк, его читать быстрее.
  • Пишите только небольшие функции, если вам нужно дублировать код.
  • Не пишите функцию с именем комментария, поместите сложную строку кода (3-4 строки) с комментарием выше; Точно так же вы можете изменить неисправный код напрямую

Это против всего, что я прочитал. Как вы обычно пишете код? Один главный большой цикл, нет маленьких функций?

Язык, который я использую, в основном Javascript. У меня действительно есть трудности с чтением, так как я удалил все свои маленькие четко названные функции и поместил все в большой цикл. Впрочем, так нравится моему боссу.

Один пример был:

// The way I would write it
if (isApplicationInProduction(headers)) {
  phoneNumber = headers.resourceId;
} else {
  phoneNumber = DEV_PHONE_NUMBER;
}

function isApplicationInProduction(headers) {
   return _.has(headers, 'resourceId');
}

// The way he would write it
// Take the right resourceId if application is in production
phoneNumber = headers.resourceId ? headers.resourceId : DEV_PHONE_NUMBER;

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

Ну, я хотел бы получить совет, какой способ / практика лучше написать чистый код?

GitCommit Виктор Б.
источник
17
Просто чтобы вы знали, Джон Кармак, вероятно, согласится с вашим боссом .
Вален
4
phoneNumber = headers.resourceId?: DEV_PHONE_NUMBER;
Джошуа
10
Подтвердите, что вы хотите работать на месте, где руководство сообщило вам, КАК выполнять вашу работу, вместо того, ЧТО нужно решить.
Константин Петрухнов
8
@rjmunro Если вам не нравится ваша работа, имейте в виду, что разработчиков меньше, чем рабочих мест. Итак, процитирую Мартина Фаулера: «Если вы не можете изменить свою организацию, измените свою организацию!» и Боссы, говорящие мне, как кодировать - это то, что я советую вам изменить.
Нильс ван Реймерсдал
10
НИКОГДА не иметь isApplicationInProduction()функции! У вас должны быть тесты, и тесты бесполезны, если ваш код ведет себя иначе, чем когда он работает. Это как намеренно иметь непроверенный / непокрытый код в производстве: это не имеет смысла.
Ронан Paixão

Ответы:

215

Взять примеры кода в первую очередь. Вы поддерживаете:

if (isApplicationInProduction(headers)) {
  phoneNumber = headers.resourceId;
} else {
  phoneNumber = DEV_PHONE_NUMBER;
}

function isApplicationInProduction(headers) {
   return _.has(headers, 'resourceId');
}

И ваш босс напишет это так:

// Take the right resourceId if application is in production
phoneNumber = headers.resourceId ? headers.resourceId : DEV_PHONE_NUMBER;

На мой взгляд, у обоих есть проблемы. Когда я читал ваш код, я сразу подумал: «Вы можете заменить ifэто троичным выражением». Затем я прочитал код вашего босса и подумал: «Почему он заменил вашу функцию комментарием?».

Я хотел бы предложить оптимальный код между двумя:

phoneNumber = isApplicationInProduction(headers) ? headers.resourceId : DEV_PHONE_NUMBER;

function isApplicationInProduction(headers) {
   return _.has(headers, 'resourceId');
}

Это дает вам лучшее из обоих миров: упрощенное тестовое выражение и комментарий заменяются тестируемым кодом.

Что касается взглядов вашего босса на дизайн кода, хотя:

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

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

Поместите все в основной большой цикл, даже если основной цикл больше 300 строк, он быстрее читается

Сканирование может быть быстрее, но, чтобы действительно «прочитать» код, вам необходимо эффективно выполнять его в своей голове. Это легко с небольшими функциями и очень, очень сложно с методами длиной в 100 строк.

Пишите только небольшие функции, если вам нужно дублировать код

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

Не пишите функцию с именем комментария, поместите сложную строку кода (3-4 строки) с комментарием выше. Таким образом, вы можете изменить код ошибки напрямую

Я действительно не могу понять причину этого, предполагая, что это действительно серьезно. Это та вещь, которую я ожидал бы увидеть в пародии на твиттер-аккаунт Эксперта для начинающих . Комментарии имеют фундаментальный недостаток: они не компилируются / интерпретируются и поэтому не могут быть проверены модулем. Код изменяется, и комментарий остается один, и вы в конечном итоге не знаете, что правильно.

Написание самодокументируемого кода сложно, и иногда необходимы дополнительные документы (даже в форме комментариев). Но мнение «дяди Боба» о том, что комментарии являются ошибкой кодирования, слишком часто встречается.

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

Дэвид Арно
источник
26
Небольшие функции тестируются на
модуле easilly
13
Quoth @ ExpertBeginner1 : «Я устал видеть тонны маленьких методов повсюду в нашем коде, так что отсюда вперед, есть 15 LOC минимум по всем методам.»
Грег Бэкон
34
«У комментариев есть фундаментальный недостаток: они не компилируются / не интерпретируются и поэтому не могут быть проверены модулем». Играя адвоката дьявола, это верно и в том случае, если вы замените «комментарии» на «имена функций».
mattecapu
11
@mattecapu, я возьму твою пропаганду и удвою ее прямо перед тобой. Любой старый разработчик мусора может колебаться в комментарии, пытаясь описать, что делает фрагмент кода. Для краткого описания этого фрагмента кода с хорошим именем функции требуется опытный коммуникатор. Лучшие разработчики являются опытными коммуникаторами, так как написание кода в первую очередь связано с общением с другими разработчиками и с компилятором в качестве второстепенной задачи. Ergo, хороший разработчик будет использовать хорошо названные функции и без комментариев; бедные разработчики скрывают свои плохие навыки за оправданиями использования комментариев.
Дэвид Арно
4
@DavidArno Все функции имеют предварительные и постусловия, вопрос в том, документируете ли вы их или нет. Если моя функция принимает параметр, который представляет собой расстояние в измеренных футах, вы должны указывать его в футах, а не в километрах. Это предварительное условие.
Йорген Фог
223

Есть и другие проблемы

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

phoneNumber = DEV_PHONE_NUMBER_WHICH_CAUSED_PROBLEMS_FOR_CUSTOMERS;

или же

phoneNumber = DEV_PHONE_NUMBER_FROM_OTHER_COUNTRY;

Хотите добавить еще больше веток?

Существенной проблемой является то, что вы в основном дублируете часть своего кода, и, таким образом, вы фактически не тестируете реальный код. Вы пишете код отладки для проверки кода отладки, но не рабочий код. Это как частичное создание параллельной кодовой базы.

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

Внедрение зависимости

Вот как должен выглядеть ваш код:

phoneNumber = headers.resourceId;

Здесь нет разветвлений, потому что логика здесь не имеет разветвлений. Программа должна вытащить номер телефона из шапки. Период.

Если вы хотите получить DEV_PHONE_NUMBER_FROM_OTHER_COUNTRYрезультат, вы должны поместить его в headers.resourceId. Один из способов сделать это - просто внедрить другой headersобъект для тестовых случаев (извините, если это неправильный код, мои навыки JavaScript немного устарели):

function foo(headers){
    phoneNumber = headers.resourceId;
}

// Creating the test case
foo({resourceId: DEV_PHONE_NUMBER_FROM_OTHER_COUNTRY});

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

значение NULL
источник
11
Я решил рассмотреть эту самую тему в своем собственном ответе, но чувствовал, что это уже достаточно долго. Так что +1 вам за это :)
Дэвид Арно
5
@DavidArno Я собирался добавить его в качестве комментария к вашему ответу, потому что вопрос был все еще заблокирован, когда я впервые прочитал его, обнаружил, к моему удивлению, что он снова был открыт, и поэтому добавил это как ответ. Возможно, следует добавить, что существуют десятки фреймворков / инструментов для автоматического тестирования. Особенно в JS, кажется, каждый день выходит новый. Может быть трудно продать это боссу, хотя.
ноль
56
@DavidArno Возможно, вам следовало бы разбить свой ответ на более мелкие ответы. ;)
Крильгар
2
@ user949300 Использование побитового ИЛИ не было бы разумно;)
curiousdannii
1
@curiousdannii Да, заметил, что уже поздно редактировать ...
user949300
59

На это нет «правильного» или «неправильного» ответа. Тем не менее, я предложу свое мнение, основанное на 36-летнем профессиональном опыте проектирования и разработки программных систем ...

  1. Не существует такого понятия, как «самодокументируемый код». Почему? Потому что это утверждение совершенно субъективно.
  2. Комментарии никогда не бывают неудачными. Что это сбой код , который не может быть понят вообще без комментариев.
  3. 300 непрерывных строк кода в одном блоке кода - это кошмар обслуживания и очень подвержен ошибкам. Такой блок убедительно свидетельствует о плохом дизайне и планировании.

Говоря непосредственно с примером, который вы предоставили ... Разумное isApplicationInProduction()(э) решение - это сделать самому. Сегодня этот тест является просто проверкой «заголовков» и может быть обработан в тернарном ( ?:) операторе. Завтра тест может быть гораздо более сложным. Кроме того, «headers.resourceId» не имеет четкого отношения к приложению «в рабочем состоянии»; Я бы сказал, что тест на такой статус должен быть отделен от базовых данных; подпрограмма сделает это, а троичная - нет. Кроме того, полезный комментарий объясняет, почему resourceId является тестом для «в производстве».

Будьте осторожны, чтобы не переборщить с «маленькими четко названными функциями». Подпрограмма должна заключать в себе идею больше, чем просто код. Я поддерживаю предложение Амона phoneNumber = getPhoneNumber(headers)и добавляю, что getPhoneNumber()следует выполнить тест «производственный статус» сisApplicationInProduction()

LiamF
источник
25
Есть такая вещь, как хорошие комментарии, которые не являются ошибкой. Тем не менее, комментарии, которые почти дословно представляют собой код, который они предположительно объясняют, или представляют собой просто пустые блоки комментариев, предшествующие методу / классу / etc. определенно провал.
jpmc26
3
Возможно иметь код, который будет меньше и легче для чтения, чем любое англоязычное описание того, что он делает, и в тех случаях, когда он делает и не обрабатывает. Кроме того, если функция извлекается в свой собственный метод, кто-то, читающий функцию, не будет знать, какие угловые случаи являются или не обрабатываются ее вызывающими, и если только имя функции не слишком многословно, кто-то, исследующий вызывающие, может не знать, какой угловой случаи обрабатываются функцией.
суперкат
7
Комментарии никогда не являются внутренне неудачами. Комментарии могут быть неудачными, и поэтому, когда они неточны. Неправильный код может быть обнаружен на нескольких уровнях, в том числе по неправильному поведению в режиме черного ящика. Неправильные комментарии могут быть обнаружены только человеческим пониманием в режиме белого ящика, путем признания того, что описаны две модели и одна из них неверна.
Тимбо
7
@ Тимбо Ты имеешь в виду: «... хотя бы один из них неверен». ;)
jpmc26
5
@immibis Если вы не можете понять, что код делает без комментариев, то, вероятно, код недостаточно ясен. Основная цель комментариев - разъяснить, почему код делает то, что делает. Это кодер, объясняющий его дизайн будущим сопровождающим. Код никогда не может дать такого рода объяснения, поэтому комментарии заполняют эти пробелы в понимании.
Грэм
47

«Сущности не должны умножаться сверх необходимости».

- Бритва Оккама

Код должен быть максимально простым. Ошибки любят скрывать между сложностями, потому что их трудно обнаружить там. Так что же делает код простым?

Небольшие единицы (файлы, функции, классы) - хорошая идея . Маленькие юниты легко понять, потому что вы должны понимать меньше вещей одновременно. Нормальные люди могут жонглировать только ~ 7 понятиями одновременно. Но размер измеряется не только в строках кода . Я могу написать как можно меньше кода, «играя в гольф» (выбирая короткие имена переменных, выбирая «умные» ярлыки, разбивая как можно больше кода на одной строке), но конечный результат не прост. Попытка понять такой код больше похожа на реверс-инжиниринг, чем на чтение.

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

Но у каждого вызова функции есть когнитивные издержки : мне нужно не просто понимать код в моем текущем фрагменте кода, я также должен понимать, как он взаимодействует с внешним кодом . Я думаю, будет справедливо сказать, что функция, которую вы извлекли, вносит в нее больше сложности, чем извлекает . Это то, что ваш начальник имел в виду под « маленькими функциями [это] боль», потому что это заставляет вас переходить к каждой маленькой функции, чтобы посмотреть, что делает код. »

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

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

Очень важной метрикой является цикломатическая сложность (сложность МакКейба). Он измеряет количество независимых путей через кусок кода. Это число растет экспоненциально с каждым условным условием. Каждое условие или цикл удваивает количество путей. Есть основания полагать, что оценка более 10 баллов слишком сложна. Это означает, что очень длинная функция, которая может иметь оценку 5, возможно, лучше, чем очень короткая и плотная функция с оценкой 25. Мы можем уменьшить сложность, выделив поток управления в отдельные функции.

Ваше условное выражение является примером части сложности, которую можно извлечь полностью:

function bigFatFunction(...) {
  ...
  phoneNumber = getPhoneNumber(headers);
  ...
}

...

function getPhoneNumber(headers) {
  return headers.resourceId ? headers.resourceId : DEV_PHONE_NUMBER;
}

Это все еще очень полезно. Я не уверен, что это существенно уменьшает сложность, потому что это условное не очень условно . В производстве он всегда будет идти по одному пути.


Сложность никогда не исчезнет. Это можно только перемешать. Много ли простых вещей проще, чем несколько больших? Это очень сильно зависит от обстоятельств. Обычно есть какая-то комбинация, которая кажется правильной. Поиск такого компромисса между различными факторами сложности требует интуиции и опыта, а также немного удачи.

Знание того, как писать очень маленькие функции и очень простые функции, является полезным навыком, потому что вы не можете сделать выбор, не зная альтернатив. Слепое следование правилам или лучшим практикам, не задумываясь о том, как они применимы к текущей ситуации, в лучшем случае приводит к средним результатам, в худшем - к культу грузового культа .

Вот где я не согласен с твоим боссом. Его аргументы не являются недействительными, но и книга о чистом кодексе неверна. Вероятно, лучше следовать указаниям вашего босса, но сам факт того, что вы думаете об этих проблемах, пытаетесь найти лучший путь, очень перспективен. По мере накопления опыта вам будет легче найти хороший факторинг для вашего кода.

(Примечание: этот ответ частично основан на мыслях из поста в блоге « Разумный код» на The Whiteboard Джимми Хоффа , который дает общее представление о том, что делает код простым.)

Амон
источник
Я генерал, мне понравился твой ответ. Я, однако, не согласен с мерой цикломатической сложности mcabes. Из того, что я видел, это не представляет собой истинную меру сложности.
Роберт Барон
27

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

Не слушай их!

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

Инструменты автоматического рефакторинга позволяют легко, просто и безопасно извлекать методы. И, пожалуйста, не принимайте всерьез людей, которые рекомендуют писать функции с> 300 строками - такие люди определенно не способны рассказать вам, как вы должны кодировать.

Док Браун
источник
53
"Не слушай их!" : учитывая тот факт, что босс попросил OP прекратить расщепление кода, OP, вероятно, следует избегать вашего совета. Даже если босс не хочет менять свои старые привычки. Также обратите внимание, что, как подчеркивалось в предыдущих ответах, код ОП и код его босса написаны плохо, и вы (намеренно или нет) не упоминаете об этом в своем ответе.
Арсений Мурзенко
10
@ArseniMourzenko: не каждый из нас должен пристегнуться перед своим боссом. Я надеюсь, что ОП достаточно взрослый, чтобы знать, когда он должен делать правильные вещи или когда он должен делать то, что говорит его начальник. И да, я не собирался вдаваться в детали примера намеренно, достаточно других ответов, уже обсуждающих эти детали.
Док Браун
8
@DocBrown Согласен. 300 строк сомнительно для всего класса. Функция 300 строк непристойна.
JimmyJames
30
Я видел много классов длиной более 300 строк, которые являются очень хорошими классами. Java настолько многословна, что без такого большого количества кода вы почти ничего не можете сделать в классе. Таким образом, «количество строк кода в классе» само по себе не является значимой метрикой, поскольку мы не считаем SLOC значимой метрикой для производительности программиста.
Роберт Харви
9
Кроме того, я видел, как мудрый совет дяди Боба неверно истолковывался и злоупотреблял настолько, что я сомневаюсь, что он полезен всем, кроме опытных программистов.
Роберт Харви
23

В вашем случае: вы хотите номер телефона. Либо очевидно, как вы получите номер телефона, тогда вы напишите очевидный код. Или не очевидно, как вы получите номер телефона, тогда вы пишете метод для него.

В вашем случае неясно, как получить номер телефона, поэтому вы пишете для него метод. Реализация не очевидна, но именно поэтому вы помещаете его в отдельный метод, поэтому вам приходится иметь дело с ним только один раз. Комментарий был бы полезен, потому что реализация не очевидна.

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

Не пишите маленькие функции. Напишите функции, которые имеют четко определенную цель и соответствуют этой четко определенной цели.

PS. Мне не нравится реализация вообще. Предполагается, что отсутствие номера телефона означает, что это версия разработчика. Таким образом, если номер телефона отсутствует в производстве, вы не только не обрабатываете его, но подставляете случайный номер телефона. Представьте, что у вас 10000 клиентов, а у 17 нет номера телефона, и у вас проблемы с производством. Находитесь ли вы в производстве или разработке, следует проверять напрямую, а не из чего-то другого.

gnasher729
источник
1
«Не пишите маленьких функций. Напишите функции, которые имеют четко определенную цель и соответствуют этой четко определенной цели». это правильный критерий для разделения кода. если функция выполняет слишком много (например, более одной) разрозненных функций , разделите ее на части. Принцип единой ответственности является руководящим принципом.
Роберт Бристоу-Джонсон
16

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

Количество строк не является полезной метрикой в ​​большинстве случаев.

300 (или даже 3000) строк совершенно тривиального чисто последовательного кода (программа установки или что-то в этом роде) редко представляет собой проблему (но может быть лучше автоматически сгенерирован или в виде таблицы данных или чего-то подобного), 100 строк вложенных циклов с множеством сложных Условия выхода и математика, которые вы можете найти в исключении Гаусса или инверсии матриц, могут быть слишком сложными для понимания.

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

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

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

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

Дэн Миллс
источник
1
Я полностью согласен - мне потребовалось бы очень сложное однострочное, чтобы обернуть его в функцию ... Я бы, конечно, не обернул тернарную / значение по умолчанию. Я обернул один вкладыш, но обычно это те сценарии оболочки, где требуется десять каналов для анализа чего-либо, а код непонятен без его запуска.
TemporalWolf
15

Не помещайте все в один большой цикл, но не делайте это слишком часто:

function isApplicationInProduction(headers) {
   return _.has(headers, 'resourceId');
}

Проблема с большим циклом состоит в том, что очень трудно увидеть его общую структуру, когда он охватывает много экранов. Поэтому старайтесь извлекать большие куски, в идеале куски, на которых лежит одна ответственность и которые можно использовать повторно.

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

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

Брэд Томас
источник
2
Я понимаю, что один булев один лайнер легко читается, но одно это действительно объясняет только то, что происходит. Я до сих пор пишу функции, которые переносят простые троичные выражения, потому что имя функции помогает объяснить причину «почему», я выполняю эту проверку состояния. Это особенно полезно, когда кто-то новый (или вы сами через 6 месяцев) должны понимать бизнес-логику.
AJ X.
14

Кажется, что вы действительно хотите это:

phoneNumber = headers.resourceId || DEV_PHONE_NUMBER

Это должно быть самоочевидным для любого, кто его читает: установите phoneNumberв значение, resourceIdесли оно доступно, или по умолчанию, DEV_PHONE_NUMBERесли это не так.

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

BlueRaja - Дэнни Пфлугхофт
источник
Это самоочевидно, что он делает (с небольшим угадыванием того, какой язык вы используете), но совершенно не очевидно, что происходит. По-видимому, разработчик предполагает, что phoneNumber хранится в разделе «resourceId» в рабочей версии, и что resourceId отсутствует в версии для разработчиков, и он хочет использовать DEV_PHONE_NUMBER в версии для разработчиков. Это означает, что номер телефона хранится в странном виде. название, и это означает, что все пойдет не так, если в
рабочей
14

Позвольте мне сказать прямо: мне кажется, что ваша среда (язык / фреймворк / дизайн классов и т. Д.) На самом деле не подходит для "чистого" кода. Вы смешиваете все возможные вещи в несколько строк кода, которые не должны быть близко друг к другу. Какое отношение имеет одна функция к знанию, что resourceId==undefозначает, что вы не работаете, что вы используете телефонный номер по умолчанию в непроизводственных системах, что идентификатор ресурса сохранен в некоторых «заголовках» и т. Д.? Я предполагаю headers, что HTTP-заголовки, так что вы даже оставляете решение о том, в какой среде вы находитесь, конечному пользователю?

Факторинг отдельных частей этого в функции не поможет вам с этой основной проблемой.

Некоторые ключевые слова для поиска:

  • развязка
  • сплоченность
  • внедрение зависимости

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

Из вашего описания («300 строк кода в« основной »функции») даже слово «функция» (вместо метода) заставляет меня предположить, что нет смысла в том, чего вы пытаетесь достичь. В этой среде программирования старой школы (т. Е. Базовое императивное программирование с небольшой структурой, конечно, без значимых классов, без шаблонов структуры классов, таких как MVC или что-то подобное), на самом деле нет особого смысла что- либо делать . Вы никогда не выйдете из отстойника без фундаментальных изменений. По крайней мере, ваш начальник, кажется, позволяет вам создавать функции, чтобы избежать дублирования кода, это хороший первый шаг!

Я знаю как тип кода, так и тип программиста, который вы описываете достаточно хорошо. Честно говоря, если бы это был сотрудник, мой совет был бы другим. Но так как это твой босс, для тебя бесполезно драться из-за этого. Мало того, что ваш босс может отвергнуть вас, но ваши дополнения кода действительно приведут к ухудшению кода, если только вы сделаете «свое дело» частично, а ваш босс (и, возможно, другие люди) продолжат, как и раньше. Это может быть лучше для конечного результата, если вы адаптируетесь к их стилю программирования (конечно, только во время работы над этой конкретной базой кода) и в этом контексте пытаетесь извлечь из этого максимум пользы.

Anoe
источник
1
Я согласен на 100%, что здесь есть неявные компоненты, которые должны быть разделены, но, не зная больше о языке / структуре, трудно понять, имеет ли смысл ОО-подход. Разделение и принцип единой ответственности важны на любом языке, от чисто функционального (например, Haskell) до чисто императивного (например, C.) Мой первый шаг - если босс это допустит - будет преобразовать основную функцию в функцию диспетчера ( как схема или оглавление), которые читаются в декларативном стиле (описывают политики, а не алгоритмы) и передают работу другим функциям.
Дэвид Леппик
JavaScript является прототипом, с первоклассными функциями. Это по сути ОО, но не в классическом смысле, поэтому ваши предположения могут быть неверными. Несколько часов просмотра видео Крокфорда на YouTube ...
Kevin_Kinsey
13

«Чистота» - одна из целей написания кода. Это не единственная цель. Еще одна достойная цель - это колокейство . Говоря неформально, словосочетание означает, что людям, пытающимся понять ваш код, не нужно прыгать повсюду, чтобы увидеть, что вы делаете. Использование хорошо названной функции вместо троичного выражения может показаться хорошим решением, но в зависимости от того, сколько таких функций у вас есть и где они расположены, эта практика может превратиться в неприятность. Я не могу сказать вам, переступили ли вы эту черту, за исключением того, что если люди жалуются, вам следует выслушать, особенно если эти люди имеют право голоса по поводу вашего статуса занятости.

user1172763
источник
2
«... кроме как сказать, что если люди жалуются, вы должны выслушать, особенно если эти люди имеют право голоса по поводу вашего статуса занятости». ИМО это действительно плохой совет. Если вы не очень плохой разработчик, которому нужно ценить любую работу, которую вы можете получить, тогда всегда применяйте принцип «если вы не можете изменить свою работу, смените работу». Никогда не чувствуй себя обязанным компании; вы нуждаетесь в них больше, чем вы нуждаетесь в них, так что уходите в лучшее место, если они не предлагают то, что вы хотите.
Дэвид Арно
4
Я немного передвигался во время моей карьеры. Я не думаю, что у меня когда-либо была работа, где я на 100% общался с моим боссом о том, как писать код. Мы люди с собственным опытом и философией. Так что я лично не оставил бы работу только потому, что было несколько стандартов кодирования, которые мне не нравились. (Кажется, что менеджеры соглашений об именовании, которым нужно сгибать пальцы, особенно противоречат тому, как я буду кодировать, если оставить их на своих устройствах.) Но вы правы, достойному программисту не нужно слишком беспокоиться о простом пребывании на работе. ,
user1172763
6

Использование небольших функций, в общем, хорошая практика. Но в идеале я считаю, что введение функции должно либо разделять большие логические блоки, либо уменьшать общий размер кода, высушивая его. Приведенный вами пример делает код длиннее и требует больше времени для чтения разработчиком, в то время как короткая альтернатива не объясняет, что "resourceId"значение присутствует только в производстве. Нечто простое, подобное этому, легко забыть и запутаться при попытке работать с ним, особенно если вы все еще плохо знакомы с базой кода.

Я не скажу, что вы должны использовать троичный, некоторые люди, с которыми я работал, предпочитают немного дольше if () {...} else {...}, это в основном личный выбор. Я предпочитаю «одна строка делает один подход», но я бы в основном придерживался того, что обычно использует кодовая база.

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

// NOTE "resourceId" not present in dev build, use test data
let isProduction = 'resourceId' in headers;
let phoneNumber = isProduction ? headers.resourceId : DEV_PHONE_NUMBER;

Я также хотел бы сказать, что если кодовая база распространяется на 300 строковых функций, то она нуждается в некотором подразделении. Но я бы посоветовал использовать более широкие штрихи.

nepeo
источник
5

Пример кода, который вы дали, ваш начальник ПРАВИЛЬНО. В этом случае лучше использовать одну четкую линию.

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

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

Фил М
источник
1
«накладные расходы на функции»: это зависит от компилятора. «затенение»: OP не указал, является ли это единственным или лучшим способом проверки этого свойства; Вы не можете знать наверняка, либо. «сложная логика спагетти»: где? «Потенциал для мертвых функций»: такой анализ мертвых кодов является малоизвестным плодом, а набор инструментов разработки, в которых его нет, незрел.
Rhymoid
Ответы были более сфокусированы на преимуществах, я только хотел указать на недостатки. Вызов такой функции, как sum (a, b), всегда будет обходиться дороже, чем «a + b» (если функция не встроена компилятором). Остальные недостатки демонстрируют, что чрезмерная сложность может привести к собственному набору проблем. Плохой код - это плохой код, и только потому, что он разбит на меньшие байты (или содержится в цикле из 300 строк), это не означает, что его легче проглотить.
Фил М
2

Я могу представить как минимум два аргумента в пользу длинных функций:

  • Это означает, что у вас много контекста вокруг каждой строки. Способ формализовать это: нарисовать график потока управления вашего кода. В вершине (~ = линия) между входом и выходом из функции вы знаете все входящие ребра. Чем длиннее функция, тем больше таких вершин.

  • Множество небольших функций означает, что существует более крупный и сложный граф вызовов. Выберите случайную строку в случайной функции и ответьте на вопрос "в каком контексте (ах) выполняется эта строка?" Это становится тем сложнее, чем крупнее и сложнее граф вызовов, потому что вам нужно смотреть на большее количество вершин в этом графе.

Есть также аргументы против длинных функций - на ум приходит юнит-тестируемость. Используйте t̶h choosinge̶ ̶f othero̶r̶c̶e̶ свой опыт при выборе между одним и другим.

Примечание: я не говорю, что ваш начальник прав, только то, что его точка зрения не может быть полностью лишена ценности.


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


Комментируя ответ Дэвида Арно :

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

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

Название ясно показывает, что означает возвращаемое значение , но ничего не говорит о последствиях выполнения кода (= что делает код ). Имена (только) передают информацию о намерении , код передает информацию о поведении (из чего иногда могут быть выведены части намерения).

Иногда вам нужно одно, иногда другое, поэтому это наблюдение не создает одностороннего универсально действующего правила принятия решений.

Поместите все в основной большой цикл, даже если основной цикл больше 300 строк, он быстрее читается

Сканирование может быть быстрее, но, чтобы действительно «прочитать» код, вам необходимо эффективно выполнять его в своей голове. Это легко с небольшими функциями и очень, очень сложно с методами длиной в 100 строк.

Я согласен, что вы должны выполнить это в своей голове. Если у вас есть 500 линий функциональности в одной большой функции по сравнению с множеством мелких функций, мне не понятно, почему это становится проще.

Предположим, что в экстремальном случае 500 строк кода с прямым побочным эффектом и вы хотите знать, произойдет ли эффект A до или после эффекта B. В большом случае функции используйте Page Up / Down, чтобы найти две строки, а затем сравнить номера строк. В случае многих маленьких функций вы должны помнить, где в дереве вызовов происходят эффекты, и если вы забыли, вам нужно потратить нетривиальное количество времени, чтобы заново открыть структуру этого дерева.

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

(*) По крайней мере, я честен об этом ;-)

Еще раз, я думаю, что оба подхода имеют свои сильные и слабые стороны.

Пишите только небольшие функции, если вам нужно дублировать код

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

Заинтересованы ли вы в том, «как» или «что», является функцией цели, для которой вы читаете код (например, получение общей идеи или отслеживание ошибки). Цель, для которой вы читаете код, недоступна при написании программы, и вы, скорее всего, будете читать код для разных целей; разные решения будут оптимизированы для разных целей.

Тем не менее, это та точка зрения босса, с которой я, вероятно, не согласен.

Не пишите функцию с именем комментария, поместите сложную строку кода (3-4 строки) с комментарием выше. Таким образом, вы можете изменить код ошибки напрямую

Я действительно не могу понять причину этого, предполагая, что это действительно серьезно. [...] Комментарии имеют фундаментальный недостаток: они не компилируются / интерпретируются и поэтому не могут быть проверены модулем. Код изменяется, и комментарий остается один, и вы в конечном итоге не знаете, что правильно.

Компиляторы сравнивают имена только на равенство, они никогда не выдают ошибку MisleadingNameError. Кроме того, поскольку несколько сайтов вызовов могут вызывать данную функцию по имени, иногда бывает труднее и подвержено ошибкам изменять имя. Комментарии не имеют этой проблемы. Однако это несколько умозрительно; чтобы действительно решить эту проблему, возможно, понадобятся данные о том, будут ли программисты чаще обновлять вводящие в заблуждение комментарии или вводящие в заблуждение имена, а у меня этого нет.

Йонас Кёлкер
источник
-1

На мой взгляд, правильный код нужной вам функциональности:

phoneNumber = headers.resourceId || DEV_PHONE_NUMBER;

Или, если вы хотите разделить его на функцию, возможно, что-то вроде:

phoneNumber = getPhoneNumber(headers);

function getPhoneNumber(headers) {
  return headers.resourceId || DEV_PHONE_NUMBER
}

Но я думаю, что у вас есть более фундаментальная проблема с понятием «в производстве». Проблема с вашей функцией isApplicationInProductionв том, что кажется странным, что это единственное место в системе, где важно находиться в «производстве», и что вы всегда можете рассчитывать на наличие или отсутствие заголовка resourceId. Должен быть общий isApplicationInProductionметод или getEnvironmentметод, который непосредственно проверяет среду. Код должен выглядеть так:

function isApplicationInProduction() {
  process.env.NODE_ENV === 'production';
}

Тогда вы можете получить номер телефона с:

phoneNumber = isApplicationInProduction() ? headers.resourceId : DEV_PHONE_NUMBER;
rjmunro
источник
-2

Просто комментарий по двум пунктам

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

Многие редакторы (например, IntelliJ) позволят вам перейти к функции / классу простым нажатием клавиши Ctrl. Кроме того, вам часто не нужно знать детали реализации функции для чтения кода, что ускоряет чтение кода.

Я рекомендую вам рассказать своему боссу; ему понравится ваша адвокатура и он будет восприниматься как лидерство Просто будь вежливым.

noɥʇʎԀʎzɐɹƆ
источник