Длина функции

Original author: Martin Fowler
  • Translation


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


Когда я стал применять такой принцип, я развил в себе привычку писать очень маленькие функции — обычно не больше нескольких строк. Любая функция длиннее шести строк уже попахивает. Вполне обычное дело для меня — иметь функцию с одной строчкой кода. Кент Бек показал мне когда-то пример из оригинальной системы Smalltalk, и это помогло мне по-настоящему понять, что размер — это не важно. Smalltalk в те годы работал на черно-белых машинах. Если нужно было подсветить текст или графику, то приходилось реверсировать видео. Класс в Smalltalk, отвечающий за графику, содержал метод 'highlight', и в его реализации была лишь одна строка — вызов метода 'reverse'. Название метода было длиннее реализации, но это не имело значения, потому что между намерением и реализацией этого кода — большое расстояние.


Некоторые люди волнуются по поводу коротких функций, потому что их заботит влияние вызовов на производительность. Когда я был молод, это иногда имело значение, но сегодня это редкость. Оптимизирующие компиляторы часто работают лучше с короткими функциями, потому что их легче кэшировать. Как обычно, в оптимизации производительности имеют смысл в первую очередь рекомендации общего характера. Иногда правильное решение это вернуть код из функции обратно в прежнее место. Но зачастую наличие маленьких функций позволяет найти другие способы оптимизации. Я помню, когда люди были против наличия метода isEmpty для списков. Стандартным способом было aList.length == 0. Но здесь как раз тот случай, когда название функции указывает на намерение, и это может помочь с производительностью если существует более быстрый способ определения пустоты коллекции, нежели проверкой длины.


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

Share post

Comments 45

    +6
    Хотя не всегда можно уместить функцию в десяток строк, тем не менее присоединюсь к автору — компактные функции читаются легче, и с ними код выглядит аккуратней. ИМХО.
      0
      Как я думаю мысль автора в том чтобы разделить «Что и как» через название и реализацию, а все что дальше это уже «смягчение» дискуса какая она должна быть потому и потому, то есть мысль в том что она может быть какая угодно. Нет проблемы в том что не всегда можно уместить функцию — это нормально.

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

        Я представляю себе как бы два разных читателя кода. Один — босс, ему вечно некогда, он вечно торопится. Ему надо только знать, что сделано. И не важно — как. А второй — технарь, вот ему важно знать, как именно сделана конкретная вещь. Но не важно, в рамках какой большой высокоуровневой задачи. От неё вообще должно быть как можно меньше зависимостей.

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

        Так вот… О чём это я? А! «Логические» функции я стараюсь делать короткими. Это как тезисы доклада для босса. А вот «физические» — они могут быть и короткими, и длинными. Зависит от алгоритма, от предметной области — да мало ли от чего. Не всегда технарю удобно нырять по стеку всё ниже и ниже каждые 5 строчек. А босс редко спросит про детали. Ну 2, максимум 3 уровня — дальше он не полезет вникать.

        И не важно, что обычно босс и технарь — это я сам. Такая вот шиза.
          0
          Я бы сказал что это не шиза, а умение думать на разных уровнях абстракции одновременно – ключевой навык для хорошего программиста, что давно подметил Спольски :-)
        +2
        «Оптимизирующие компиляторы часто работают лучше с короткими функциями, потому что их легче кэшировать» — что имелось в виду? что значит кэшировать функции?

        + В С/С++ если мелкие функции разбросаны по разным единицам трансляции, то лишь применение IPO/LTO заинлайнит такие вызовы. А оно примеряется далеко не всегда.
          0

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


          По-моему, из контекста всё очевидно. Сомнения в том, выделять код в отдельную функцию или не выделять, возникают только тогда, когда все итоговые функции будут находиться в одном файле… и, скорее всего, даже в одном классе. Когда код нужно вызывать из других файлов, его по-любому придётся оформить в виде отдельной функции. К сожалению (к счастью), goto между файлами не переходит.

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

              Расскажите это разработчикам на node.js. То, что вы считаете плохим признаком, у них является ключевым моментом методологии разработки.

                0
                Мне их жаль. Малейшее изменение в коде такой функции и целые проекты пачками падают в бездну…
                  0

                  Именно это и произошло не так давно.

                    0
                    Малейшее исправление бага, и чинятся все зависимые проекты. Блин, это же типичное копипаста вс депенденси, причем получается что вы за копипасту. В каком мире копипаста лучше?
                      0
                      Классический вариант неправильно поставленного вопроса, на который нельзя ответить правильно.
            +2
            В принципе согласен. Но на практике могу и в несколько экранов функции писать, а уже потом по мере свободного времени заниматься рефакторингом по их декомпозиции.
              0
              Угу.
              Частенько бывает удобно набросать «монстра» абы-как — точнее «как есть», если понятно, что он должен делать.
              А потом уже существующий текст аккуратно расчленять на функции — причём в процессе это работы функции могут нарисоваться даже там, где они первоначально никак не планировались.
            • UFO just landed and posted this here
                +5
                Ну и стек. Каждый вызов функции сопровождается пушем в стек.
                  +3
                  Так вот эти проверки и вынести в отдельную функцию. Фишка в том что при анализе функции ты будешь видеть две строчки — подготовка и проверка а не 20 строк непойми какого зубодробительного мусора. Но т.к. эта функция используется только один раз то компилятор её заинлайнит и окончательный код никак не будет отличаться от изначальной — никаких лишних вызовов, использование стека и т.д.
                  Выгода в структурированности исходного кода, а не результата.
                  • UFO just landed and posted this here
                      0
                      Так в том-то и дело что разбивать предлагается на ПОНЯТНЫЕ фрагменты. Оно итак понятно что если разбивать функцию на непонятные фрагменты то будет только хуже. И если приходится входить в функцию чтобы понять что она делает — это неправильное разбиение. Ну и да, всегда попадётся крепкий орешек который не раскусить, даже алмазные буры бывает ломаются.
                    +6

                    Код в первую очередь должен быть понятным на уровне предметной области. А преждевременная оптимизация — зло. Будь то функции по 100+ строк или вынос каждого оператора в отдельную функцию.


                    Код в первую очередь должен быть читабелен человеком.

                      +1
                      Не уверен, какие именно функции автор имеет в виду (отдельные функции, функции-члены, методы?), но он, по-моему, забывает еще одну очень важную вещь, что функции, почти все, контекстно зависимые, и этот контекст необходимо передавать в функцию, либо делать глобальным.

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

                      В случае с передачей контекста в функцию придется либо оформлять его в структуру/класс/коллекцию, либо передавать в виде пачки параметров. Если на каждую функцию делать свою структуру, например, то это может вылиться в кучу структур, которые еще в дополнение к вызову метода надо будет инициализировать. А если передавать как пачку параметров, то в итоге можем получить ситуацию, когда вызов функции со всеми параметрами длиннее, чем сама функция.

                      Я, конечно, утрирую, но, думаю, смысл понятен; маленькие функции по… гмм… «методологии» автора могут в такой же степени злом, как и добром.
                        0
                        Маленькие функции, как правило, очень слабо зависят от контекста, и более того т.к. вызываются только единожды то контекст у них ровно один — то же самый что у исходного кода который завернули в эту функцию.
                        Просто берется код и сворачивается чтобы глаз не мозолил лишний раз. Не более того. Обычно это отлаженный код, который легко проверить по входным данным и результату и причин углубляться в него нет никаких.
                      0
                      Лично меня меньше всего волнуют размеры моих функций и методов, в плане искусственной границы, обозначенной каким-нибудь потолочным магическим числом 6 или 10 (почему не 11 и не 12?).

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

                      Никогда не уменьшаю длину функции в ущерб читаемости и понятности. Никогда не выношу блок кода в отдельную функцию ТОЛЬКО ради того, чтобы другая функция стала короче.

                      И лично для меня, «попахивают» не длинные функции, а советы «не делать методы длинней X строк». Можно (и нужно) дать совет не делать длинные функции, не забывать про декомпозицию и пр., но когда в таком совете появляется цифра — то на свалку такой совет.

                        –4
                        Ситуация хуже некуда: звёздный кодер придумывает себе свои личные персональные принципы, а потом сам их пропагандирует, мол, я звезда и делаю вот так, а лемминги вокруг рты откроют и поддакивают, а потом с ними говорить невозможно, лопочут неосознанные догмы и всё, vox populli. Сотни их было, Макконел, или там, suckless, прости господи. Светлая идея ценности каждого мнения сталкивается с жестокой реальностью фанатизма и долбоклюйства. Субъективный идеализм, мать его.
                          –3
                          Советую прочитать «Чистый код» Мартина;
                          Сначала кажется, что с такими мелкими методами, кода будет гораздо больше и будет множество методов. Но в итоге все сворачивается и становится простым и лаконичным.
                          А выносить часть кода в функцию, только ради избавления копипасты — это в корне не верно.
                            +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;
                            }
                            


                            Так что мне кажется выносить функцию которую используешь только один раз — блажь.
                              –7
                              Ну почему же, стало гораздо понятней — в конечной функции стало меньше воды, просто надо остальные функции вынести в другой файл долой с глаз чтобы не мешались — они нужны будут только в крайнем случае, когда понадобится посмотреть что же всё-таки происходит в коде.
                                0
                                По-моему, вы перегнули палку. Совершенно ясно, что не нужна отдельная функция для создания 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;
                                }
                                
                                
                                  0
                                  Очень часто вижу подобное от новых адептов новейшей революционной парадигмы функционального программирования. Они оборачивают в микрофункции ВСЕ. Даже небо и даже аллаха.
                                  0
                                  Из названия функции видно, что её можно разбить на три:
                                  • Request
                                  • Parse
                                  • Save

                                  Рано или поздно нам придётся писать ещё одну бизнес функцию, в которой будет получение данных и их сохранение.
                                  +1
                                  Семь раз написать — один обрезать
                                    0
                                    тестировать проще.
                                      +1
                                      Что лучше — 10000 файлов в одной папке или 10000 папок с одним файлом в каждой? Ни то, ни другое, 100 на 100 оптимальный выбор. Надо понимать, что убирая сложность с одного уровня абстракции мы переносим его на другой.
                                        –1
                                          0
                                          Все верно, но что считать за элемент — строку или блок? Нам, в принципе, не нужно держать в памяти непосредственно все строки кода, надо держать суть блоков — здесь 5 строк вычисляют среднее квадратичное, здесь 5 строк выполняют нормализацию, здесь 5 строк выводят данные. Это три элемента, а не 15.
                                            0
                                            Судя по вики, если в методе 7+-2 строк, то мозг будет оперировать строками.
                                            Если больше, то блоками, т.е. сделает виртуальный рефакторинг и будет воспринимать блок кода как заинлайненную функцию.
                                              0
                                              Это значит, что при правильном структурировании (разбиении на блоки) мы можем нормально оперировать 49 строками в среднем, в противовес куче функций по 2-3 строки.
                                                0
                                                49 — это примерно один экран в 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)
                                                }
                                                
                                                В этой функции столько связей, что она практически нечитаема.
                                                  0
                                                  Справедливости ради, она более нечитаема (пмсм) из-за коротких имен, а не количества связей. А так верно, надо смотреть по месту.
                                            0
                                            Кстати, в книге Барбары Оакли Думай как математик утверждается что данное утверждение устарело и сейчас считается что в рабочей памяти может содержаться 4 порции информации. Вроде бы по тексту даже ссылки на конкретные исследования были.
                                              0
                                              Спасибо за ссылку. Надо почитать…
                                            –1
                                            Опять любители дробить цельный понятный код на куски с сокрытием сути за якобы понятным именем разбушевались.
                                              0
                                              Разбиение на куски привносит два жирных плюса:

                                              1) автодокументирование, пояснять большой кусок кода комментариями больше становится не нужно, название функции прекрасно само говорит за себя

                                              2) тестирование, маленькие куски выполняют очень мало работы, следовательно покрытие тестами становится весьма тривиальной задачей

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


                                                Вообще не факт. Вот есть у вас функция «GetHeader», ну получает она какой-то заголовок, и что? То, что вместо комментария //get header list for generating report column headers вы назвали кусок кода GetHeader, не делает этот код понятнее ни на грамм.

                                                2) тестирование, маленькие куски выполняют очень мало работы, следовательно покрытие тестами становится весьма тривиальной задачей

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

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

                                            Only users with full accounts can post comments. Log in, please.