На протяжении своей карьеры я слышал множество аргументов о длине функции. Более глубокий вопрос — когда код нужно выносить в отдельную функцию? Иногда рекомендации основаны на размере, например, функция должна помещаться на экране. Другие основаны на повторном использовании — любой код, используемый больше одного раза, должен быть вынесен в отдельную функцию. Но если код используется лишь один раз, то можно его оставить на месте. Мне кажется, что большим смыслом обладает аргумент о разделении намерения и реализации. Если нужно потратить время на поиски фрагмента кода чтобы понять, что он делает, то нужно вынести его в функцию и дать ей такое имя, которое отвечает на вопрос "что". Тогда в следующий раз смысл функции сразу будет очевидным, и в большинстве случаев вас не будет волновать то, как функция выполняет свою работу. Иными словами — что происходит в теле функции.
Когда я стал применять такой принцип, я развил в себе привычку писать очень маленькие функции — обычно не больше нескольких строк. Любая функция длиннее шести строк уже попахивает. Вполне обычное дело для меня — иметь функцию с одной строчкой кода. Кент Бек показал мне когда-то пример из оригинальной системы Smalltalk, и это помогло мне по-настоящему понять, что размер — это не важно. Smalltalk в те годы работал на черно-белых машинах. Если нужно было подсветить текст или графику, то приходилось реверсировать видео. Класс в Smalltalk, отвечающий за графику, содержал метод 'highlight', и в его реализации была лишь одна строка — вызов метода 'reverse'. Название метода было длиннее реализации, но это не имело значения, потому что между намерением и реализацией этого кода — большое расстояние.
Некоторые люди волнуются по поводу коротких функций, потому что их заботит влияние вызовов на производительность. Когда я был молод, это иногда имело значение, но сегодня это редкость. Оптимизирующие компиляторы часто работают лучше с короткими функциями, потому что их легче кэшировать. Как обычно, в оптимизации производительности имеют смысл в первую очередь рекомендации общего характера. Иногда правильное решение это вернуть код из функции обратно в прежнее место. Но зачастую наличие маленьких функций позволяет найти другие способы оптимизации. Я помню, когда люди были против наличия метода isEmpty для списков. Стандартным способом было aList.length == 0. Но здесь как раз тот случай, когда название функции указывает на намерение, и это может помочь с производительностью если существует более быстрый способ определения пустоты коллекции, нежели проверкой длины.
Маленькие функции вроде этой работают только если названия достаточно хороши, так что нужно уделять внимание этому. Со временем ваш навык будет улучшаться, и такой подход может сильно повышать само-документированность кода. Функции более высокого уровня могут читаться как история, и читатель выбирает, в какие функции углубиться если нужно узнать подробности.
Комментарии (45)
RainM
02.12.2016 13:28+2«Оптимизирующие компиляторы часто работают лучше с короткими функциями, потому что их легче кэшировать» — что имелось в виду? что значит кэшировать функции?
+ В С/С++ если мелкие функции разбросаны по разным единицам трансляции, то лишь применение IPO/LTO заинлайнит такие вызовы. А оно примеряется далеко не всегда.Shamov
02.12.2016 19:07А если функции разбросаны по разным проектам, которые собираются независимо друг от друга на разных машинах, то такие вызовы не сможет заинлайнить вообще никакая оптимизация.
По-моему, из контекста всё очевидно. Сомнения в том, выделять код в отдельную функцию или не выделять, возникают только тогда, когда все итоговые функции будут находиться в одном файле… и, скорее всего, даже в одном классе. Когда код нужно вызывать из других файлов, его по-любому придётся оформить в виде отдельной функции. К сожалению (к счастью), goto между файлами не переходит.
Alexeyslav
05.12.2016 10:05Если микро-функция используется в нескольких проектах, то это очень очень плохой признак! В нормальной ситуации такого не должно быть и в здравом уме в голову не придёт — такие микро-функции не должны идти в экспортную секцию модуля, только для внутреннего применения в пределах модуля.
Shamov
05.12.2016 10:59Расскажите это разработчикам на node.js. То, что вы считаете плохим признаком, у них является ключевым моментом методологии разработки.
Alexeyslav
05.12.2016 15:24Мне их жаль. Малейшее изменение в коде такой функции и целые проекты пачками падают в бездну…
PsyHaSTe
10.12.2016 15:05Малейшее исправление бага, и чинятся все зависимые проекты. Блин, это же типичное копипаста вс депенденси, причем получается что вы за копипасту. В каком мире копипаста лучше?
Alexeyslav
11.12.2016 22:04Классический вариант неправильно поставленного вопроса, на который нельзя ответить правильно.
leremin
02.12.2016 14:31+2В принципе согласен. Но на практике могу и в несколько экранов функции писать, а уже потом по мере свободного времени заниматься рефакторингом по их декомпозиции.
aso
02.12.2016 15:17Угу.
Частенько бывает удобно набросать «монстра» абы-как — точнее «как есть», если понятно, что он должен делать.
А потом уже существующий текст аккуратно расчленять на функции — причём в процессе это работы функции могут нарисоваться даже там, где они первоначально никак не планировались.
Ivan_83
02.12.2016 15:24+10Автор просто пишет всякую фигню в вакууме на каком то выскоуровневом языке/фреймворке и потому совет для него актуален: никаких длинных слов и функции в 6 строк максимум реальны.
Те для функций реализующих логику оно применимо, а для функций где идёт обработка данных нет.
На практике многие вещи просто не делятся не куски или это деление не имеет смысла.
Например, любая функция обработки блока в любом крипто алгоритме для хэша или симметричного шифрования заведомо много больше 6 строк, если конечно не в падать в маразм и не пытаться это всё спрятать под ковёр размазав по дефайнам/инлайнам или ещё как то (от чего скорее всего упадёт производительность) или не сотворив длинных строк на несколько широкоэкранных мониторов.
Ну или вот деление длинных чисел — нет же никакого смысла и логики пытаться функцию деления расчленить, и там только предварительная подготовка и проверки строк 20-30.
Аналогично любой ip_input(), tcp_input() в любой ОС или либе это дохрена строчек кода и ещё больше комментариев поясняющих чего тут вообще происходит и зачем.
И тд.
Автор совсем не заботится о производительности, все эти аргументы что вызовы ничего не стоят — смешны, когда у тебя длинный цикл и в нем что то вызывается что гарантированно не заинлайнилось.
Те про конвеер комманд автор не в курсе и что от переходов он флюшится.Alexeyslav
02.12.2016 16:47+3Так вот эти проверки и вынести в отдельную функцию. Фишка в том что при анализе функции ты будешь видеть две строчки — подготовка и проверка а не 20 строк непойми какого зубодробительного мусора. Но т.к. эта функция используется только один раз то компилятор её заинлайнит и окончательный код никак не будет отличаться от изначальной — никаких лишних вызовов, использование стека и т.д.
Выгода в структурированности исходного кода, а не результата.Ivan_83
05.12.2016 02:33Вот оно: http://www.netlab.linkpc.net/download/software/SDK/core/include/math_bn.h
bn_div(bn_p bn, bn_p d, bn_p remainder) {
…
Вот тебе зубодробительного мусора, наслаждайся ))))
Мне нахер не нужно видеть две строчки, смысл которых чтобы понять нужно прыгать по коду как сайгак по горам.
Вся ценность в том, что код должен читаться как книга, без частых отсылок куда по дальше (функции или гото).
В моих примерах коменты скудноваты, их скорее всего не хватит чтобы въехать человеку со стороны, хотя тема та ещё, тот же ip_input() в любой ОС в разы проще ибо там 100500 проверок и обработок простых понятных вещей.
Я не вижу никакой структурированности в предложениях разбить функцию деления большого числа на непонятные фрагменты, которые больше нигде использоваться никогда не будут, или тоже самое с ip_input(), tcp_input() во фре, линухе…
Кода там в принципе меньше сделать нельзя, а растаскивание по функциям — просто раздует код ещё больше и сделает его не удобным для чтения и понимания.
Для примера представь что я в своём тексте заменил все слога на числа и тебе чтобы понять что я написал нужно сидеть каждую циферку искать с табличке слога из которых составлять слова обратно.
Те делать функции короткими потому что так понятно идиотам — это идиотизм в вакууме.
Критерии кода который легко понять:
— объём
— комменты
— оформление / codestyle
— понятные, легко запоминающиеся названия функций и переменных
— сравнительно не большой объём «словаря» из имён используемых в коде функций и переменных
Собственно выше я уже писал что далеко не весь код имеет смысл нарезать на тонкие куски по 1-5 строк, именно потому что:
— они больше нигде не используются (ибо большая функция уже атомарна — самый низкий и элементарный уровень и делает ровно одно простое действие)
— придётся лишний раз прыгать в них чтобы понять что они делают
те выгоды ноль только трата времени и сил.
Например вот здесь: http://www.netlab.linkpc.net/download/software/SDK/core/include/ecdsa.h
функции сильно меньше, но они активно вызывают другие довольно жирные функции для расчётов.
Вот так выглядит настоящий работающий код для расчётов в ассиметричной криптографии (ECDSA+ГОСТ2001+ГОСТ2012) на эллиптических кривых.
Если хочешь ещё поспорить — можешь переписать по своей методе, посмотрим насколько оно будет рабочее и понятное, а заодно и как быстро считать будет :)Alexeyslav
05.12.2016 10:10Так в том-то и дело что разбивать предлагается на ПОНЯТНЫЕ фрагменты. Оно итак понятно что если разбивать функцию на непонятные фрагменты то будет только хуже. И если приходится входить в функцию чтобы понять что она делает — это неправильное разбиение. Ну и да, всегда попадётся крепкий орешек который не раскусить, даже алмазные буры бывает ломаются.
wert_lex
02.12.2016 17:04+6Код в первую очередь должен быть понятным на уровне предметной области. А преждевременная оптимизация — зло. Будь то функции по 100+ строк или вынос каждого оператора в отдельную функцию.
Код в первую очередь должен быть читабелен человеком.
Workanator
02.12.2016 18:38+1Не уверен, какие именно функции автор имеет в виду (отдельные функции, функции-члены, методы?), но он, по-моему, забывает еще одну очень важную вещь, что функции, почти все, контекстно зависимые, и этот контекст необходимо передавать в функцию, либо делать глобальным.
Глобальный контекст (глобальные переменные) — это палка о двух концах, где-то оправдан, а где-то вреден. В любом случае, с глобальным контекстом читающему код уж точно сладко не придется, потому что придется листать вверх-вниз, чтобы понять что куда присвоилось и когда, и зачем.
В случае с передачей контекста в функцию придется либо оформлять его в структуру/класс/коллекцию, либо передавать в виде пачки параметров. Если на каждую функцию делать свою структуру, например, то это может вылиться в кучу структур, которые еще в дополнение к вызову метода надо будет инициализировать. А если передавать как пачку параметров, то в итоге можем получить ситуацию, когда вызов функции со всеми параметрами длиннее, чем сама функция.
Я, конечно, утрирую, но, думаю, смысл понятен; маленькие функции по… гмм… «методологии» автора могут в такой же степени злом, как и добром.Alexeyslav
05.12.2016 10:16Маленькие функции, как правило, очень слабо зависят от контекста, и более того т.к. вызываются только единожды то контекст у них ровно один — то же самый что у исходного кода который завернули в эту функцию.
Просто берется код и сворачивается чтобы глаз не мозолил лишний раз. Не более того. Обычно это отлаженный код, который легко проверить по входным данным и результату и причин углубляться в него нет никаких.
Alex_T666
02.12.2016 16:08Лично меня меньше всего волнуют размеры моих функций и методов, в плане искусственной границы, обозначенной каким-нибудь потолочным магическим числом 6 или 10 (почему не 11 и не 12?).
Но подсказки моего анализатора кода по поводу слишком длинного метода принимаю во внимание всегда, как знак того что может быть здесь имеет смысл порефакторить чуток. Но если смысла нет, у меня никогда не зудит, что метод длинный.
Никогда не уменьшаю длину функции в ущерб читаемости и понятности. Никогда не выношу блок кода в отдельную функцию ТОЛЬКО ради того, чтобы другая функция стала короче.
И лично для меня, «попахивают» не длинные функции, а советы «не делать методы длинней X строк». Можно (и нужно) дать совет не делать длинные функции, не забывать про декомпозицию и пр., но когда в таком совете появляется цифра — то на свалку такой совет.
pengyou
02.12.2016 16:13-4Ситуация хуже некуда: звёздный кодер придумывает себе свои личные персональные принципы, а потом сам их пропагандирует, мол, я звезда и делаю вот так, а лемминги вокруг рты откроют и поддакивают, а потом с ними говорить невозможно, лопочут неосознанные догмы и всё, vox populli. Сотни их было, Макконел, или там, suckless, прости господи. Светлая идея ценности каждого мнения сталкивается с жестокой реальностью фанатизма и долбоклюйства. Субъективный идеализм, мать его.
arTk_ev
02.12.2016 16:39-3Советую прочитать «Чистый код» Мартина;
Сначала кажется, что с такими мелкими методами, кода будет гораздо больше и будет множество методов. Но в итоге все сворачивается и становится простым и лаконичным.
А выносить часть кода в функцию, только ради избавления копипасты — это в корне не верно.
Fen1kz
02.12.2016 16:40+7Ну не знаю. Много раз сталкивался с разделением функций ради разделения функций, читать отвратительно. Вместо простого
public void requestSomethingParseAndSave(String parameter) { List<Integer> data = this.getAPI().getData(parameter); this.getObservers().notify('data received') List<Integer> newData = new ArrayList<>(); for (Integer i : data) { newData.push(i + 15); } this.getObservers().notify('data parsed', newData) this.getAPI().saveData(newData); }
Получается что-нибудьвот такоеprivate void notifyObserversThatDataIsReceived() { this.getObservers().notify('data received') } private List<Integer> getNewData() { return new ArrayList<>(); } private List<Integer> parseSomething(List<Integer> data) { // TODO rewrite, too many lines List<Integer> newData = getNewData(); for (Integer i : data) { newData.push(doParsing(i)); } return newData; } public void requestSomethingParseAndSave(String parameter) { List<Integer> data = this.requestSomething(parameter); List<Integer> newData = this.parseSomething(data); this.saveData(newData); } private Integer doParsing(Integer i) { return i + 15; } private List<Integer> getData(String parameter) { return this.getAPI().getData(parameter); } private void saveData(List<Integer> newData) { this.getObservers().notify('data parsed', newData) this.getAPI().saveData(newData); } private List<Integer> requestSomething() { List<Integer> data = this.getData(parameter) this.notifyObserversThatDataIsReceived() return data; }Alexeyslav
02.12.2016 16:54-7Ну почему же, стало гораздо понятней — в конечной функции стало меньше воды, просто надо остальные функции вынести в другой файл долой с глаз чтобы не мешались — они нужны будут только в крайнем случае, когда понадобится посмотреть что же всё-таки происходит в коде.
naething
02.12.2016 20:55По-моему, вы перегнули палку. Совершенно ясно, что не нужна отдельная функция для создания ArrayList'а.
Конкретно в вашем примере может иметь смысл вынести логику преобразования элемента в отдельную функцию. А для преобразования массивов в Java 8 есть стандартные средства.
public void requestSomethingParseAndSave(String parameter) { List<Integer> data = this.getAPI().getData(parameter); this.getObservers().notify('data received') List<Integer> newData = data.stream().map(MyClass::parseData).collect(Collectors.toList()); this.getObservers().notify('data parsed', newData) this.getAPI().saveData(newData); } private static int parseData(int x) { return x + 15; }
andreysmind
04.12.2016 11:36Очень часто вижу подобное от новых адептов новейшей революционной парадигмы функционального программирования. Они оборачивают в микрофункции ВСЕ. Даже небо и даже аллаха.
Deosis
03.12.2016 11:03Из названия функции видно, что её можно разбить на три:
- Request
- Parse
- Save
Рано или поздно нам придётся писать ещё одну бизнес функцию, в которой будет получение данных и их сохранение.
michael_vostrikov
03.12.2016 06:09+1Что лучше — 10000 файлов в одной папке или 10000 папок с одним файлом в каждой? Ни то, ни другое, 100 на 100 оптимальный выбор. Надо понимать, что убирая сложность с одного уровня абстракции мы переносим его на другой.
tmk826
03.12.2016 12:11-1Вроде как этот вопрос уже давно решен:
Магическое число семь плюс-минус два
The Magical Number Seven, Plus or Minus TwoMacIn
03.12.2016 22:57Все верно, но что считать за элемент — строку или блок? Нам, в принципе, не нужно держать в памяти непосредственно все строки кода, надо держать суть блоков — здесь 5 строк вычисляют среднее квадратичное, здесь 5 строк выполняют нормализацию, здесь 5 строк выводят данные. Это три элемента, а не 15.
Deosis
05.12.2016 07:21Судя по вики, если в методе 7+-2 строк, то мозг будет оперировать строками.
Если больше, то блоками, т.е. сделает виртуальный рефакторинг и будет воспринимать блок кода как заинлайненную функцию.MacIn
06.12.2016 21:08Это значит, что при правильном структурировании (разбиении на блоки) мы можем нормально оперировать 49 строками в среднем, в противовес куче функций по 2-3 строки.
Deosis
07.12.2016 07:4549 — это примерно один экран в IDE. Но мы работаем не со сферическими функциями в вакууме.
В функции используются параметры/поля класса, которые тоже требуют внимания.
Также можно посмотреть на функцию обработки сообщений от ОС. Она обычно выглядит как свитч по одному параметру, где case блоки независимы. С такой функцией можно работать, даже если она состоит из сотен строк.
Если блоки выстроены в конвейер (каждый блок оперирует только с переменными, записанными предыдущим блоком), то их может быть больше 7, но это синтетический пример.
Поэтому наличие связей между блоками также влияет на количество оперируемых строк.
Даже функцию на 3 строки можно сделать нечитаемой:
В этой функции столько связей, что она практически нечитаема.auto Process(auto var1, auto var2, ..., auto var7) { Prepare(var1, var2, var4, var6, var7); Update(var2, var3, var4, var5, var6); Save(var1, var3, var5, var7) }MacIn
07.12.2016 18:37Справедливости ради, она более нечитаема (пмсм) из-за коротких имен, а не количества связей. А так верно, надо смотреть по месту.
Neikist
04.12.2016 21:54Кстати, в книге Барбары Оакли Думай как математик утверждается что данное утверждение устарело и сейчас считается что в рабочей памяти может содержаться 4 порции информации. Вроде бы по тексту даже ссылки на конкретные исследования были.
MacIn
03.12.2016 17:40-1Опять любители дробить цельный понятный код на куски с сокрытием сути за якобы понятным именем разбушевались.
hVostt
03.12.2016 22:43Разбиение на куски привносит два жирных плюса:
1) автодокументирование, пояснять большой кусок кода комментариями больше становится не нужно, название функции прекрасно само говорит за себя
2) тестирование, маленькие куски выполняют очень мало работы, следовательно покрытие тестами становится весьма тривиальной задачей
Если принять это во внимание, то можно отбросить фломастерный вопрос по поводу чистоты и приглядности кода.MacIn
03.12.2016 22:54+11) автодокументирование, пояснять большой кусок кода комментариями больше становится не нужно, название функции прекрасно само говорит за себя
Вообще не факт. Вот есть у вас функция «GetHeader», ну получает она какой-то заголовок, и что? То, что вместо комментария //get header list for generating report column headers вы назвали кусок кода GetHeader, не делает этот код понятнее ни на грамм.
2) тестирование, маленькие куски выполняют очень мало работы, следовательно покрытие тестами становится весьма тривиальной задачей
Если оно при этом снижает читаемость, то это сомнительный tradeoff. Я не против тестирования, я против догмы «куча маленьких функций заведомо понятнее и лучше читается».
amakhrov
04.12.2016 22:402) тестирование, маленькие куски выполняют очень мало работы, следовательно покрытие тестами становится весьма тривиальной задачей
Если эти маленькие функции — приватные, то на тестирование это вообще не влияет. (это случай, когда код из длинного публичного метода в классе вынесли в несколько приватных в том же классе).
claygod
Хотя не всегда можно уместить функцию в десяток строк, тем не менее присоединюсь к автору — компактные функции читаются легче, и с ними код выглядит аккуратней. ИМХО.
deppkind
Как я думаю мысль автора в том чтобы разделить «Что и как» через название и реализацию, а все что дальше это уже «смягчение» дискуса какая она должна быть потому и потому, то есть мысль в том что она может быть какая угодно. Нет проблемы в том что не всегда можно уместить функцию — это нормально.
Я не спорю с вами — это мое имхо что написано, с чем я собственно полностью согласен на уровне — сначала идет definition того что делается, а уже другой очередью идет реализация — как бы разные процессы мыследеятельности. Вот об этом «заборе» мне кажется он и пишет — а что за забором это уже вообще не имеет значения без точечного контекста.
beetleinweb
Я стараюсь разделять свои функции на два типа — «логические» (не имеет ничего общего с логическими операциями) и «физические» (разумеется, ничего общего с физикой).
Я представляю себе как бы два разных читателя кода. Один — босс, ему вечно некогда, он вечно торопится. Ему надо только знать, что сделано. И не важно — как. А второй — технарь, вот ему важно знать, как именно сделана конкретная вещь. Но не важно, в рамках какой большой высокоуровневой задачи. От неё вообще должно быть как можно меньше зависимостей.
Я знаю, в разных умных книжках эти вещи по разному называются. «Бизнес-логика» и «низкоуровневая реализация», или ещё как-нибудь. Но практически все авторы имеют ввиду если не одно и то же, то нечто близкое. В больших сложных проектах уровней может быть и побольше, этакая фрактальная структура.
Так вот… О чём это я? А! «Логические» функции я стараюсь делать короткими. Это как тезисы доклада для босса. А вот «физические» — они могут быть и короткими, и длинными. Зависит от алгоритма, от предметной области — да мало ли от чего. Не всегда технарю удобно нырять по стеку всё ниже и ниже каждые 5 строчек. А босс редко спросит про детали. Ну 2, максимум 3 уровня — дальше он не полезет вникать.
И не важно, что обычно босс и технарь — это я сам. Такая вот шиза.
semper
Я бы сказал что это не шиза, а умение думать на разных уровнях абстракции одновременно – ключевой навык для хорошего программиста, что давно подметил Спольски :-)