Когда функция слишком длинная? [закрыто]

131

35 строк, 55 строк, 100 строк, 300 строк? Когда начинать разбирать? Я спрашиваю, потому что у меня есть функция с 60 строками (включая комментарии), и я думал о том, чтобы разбить ее на части.

long_function(){ ... }

в:

small_function_1(){...}
small_function_2(){...}
small_function_3(){...}

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

Когда бы вы разбили функцию на более мелкие? Зачем?

  1. Методы должны делать только одну логическую вещь (думать о функциональности)
  2. Вы должны уметь объяснить метод одним предложением
  3. Он должен соответствовать высоте вашего дисплея
  4. Избегайте ненужных накладных расходов (комментарии, указывающие на очевидное ...)
  5. Модульное тестирование проще для небольших логических функций
  6. Проверьте, можно ли повторно использовать часть функции другими классами или методами.
  7. Избегайте чрезмерной связи между классами
  8. Избегайте глубоко вложенных управляющих структур

Спасибо всем за ответы , отредактируйте список и проголосуйте за правильный ответ, я выберу этот;)

Я занимаюсь рефакторингом с учетом этих идей :)

58163
источник
В вашем вопросе опечатка, я думаю, вы имели в виду «Когда функция слишком длинная?».
Tom
1
Вы неправильно формулируете вопрос, формулируя его в виде строк кода. Определяющие факторы не измеряются строками кода.
dkretz
этот вопрос может усложниться в зависимости от кода и языка. может ты сможешь опубликовать это.
Рэй Тайек,
Если соблюдается принцип единой ответственности - делай. Обычно я чувствую необходимость делать заголовок или для каждых 20 строк кода, что заставляет меня абстрагироваться от него и назвать этот фрагмент функцией со значимым именем вместо заголовка главы.
Евгений Афанасьев

Ответы:

75

Для этого нет никаких жестких и жестких правил. Обычно мне нравятся мои методы, которые позволяют «делать что-то одно». Так что, если он захватывает данные, затем что-то делает с этими данными, а затем записывает их на диск, я бы разделил захват и запись на отдельные методы, так что мой «основной» метод просто содержит «что-то делать».

Это «что-то делать» все равно может состоять из нескольких строк, поэтому я не уверен, что количество строк - правильный показатель для использования :)

Изменить: это одна строка кода, которую я отправил по почте на прошлой неделе (чтобы доказать свою точку зрения ... это не то, к чему я привык :)) - Я бы точно не хотел, чтобы 50-60 этих плохих парней в моем методе : D

return level4 != null ? GetResources().Where(r => (r.Level2 == (int)level2) && (r.Level3 == (int)level3) && (r.Level4 == (int)level4)).ToList() : level3 != null ? GetResources().Where(r => (r.Level2 == (int)level2) && (r.Level3 == (int)level3)).ToList() : level2 != null ? GetResources().Where(r => (r.Level2 == (int)level2)).ToList() : GetAllResourceList();
Стивен Роббинс
источник
1
LOL Ну, я мог бы удалить все пробелы в моем методе, и это была бы только одна очень длинная строка, а не длинная функция. Сделать одну вещь, это, вероятно, ответ, чем, спасибо
@Movaxes Этот фрагмент кода, который я опубликовал, представляет собой одно выражение, а не просто множество строк в одной строке ... там нет точек с запятой :) Я мог бы каждый раз расширять GetResources (), чтобы сделать его еще более злым: P
Стивен Роббинс,
Да, в этом есть смысл. Почему бы просто не взять весь исходный файл и поместить его в одну строку. Я имею в виду, тогда вы действительно станете «ниндзя» Web 2.0 :)
BobbyShaftoe
Я помню, как в старых журналах (я говорю о старых BBC Micro) у них были «программы из 10 строк», в которых было несколько утверждений в каждой строке, вплоть до максимальной длины, которую могла выдержать BBC .. они всегда были настоящей болью ввести: D
Стивен Роббинс,
6
Мне нравится концепция функции, выполняющей только одно, .... но. Если у вас есть функция, которая выполняет 10 действий, и вы перемещаете 9 из этих функций в отдельные функции, которые все еще вызываются оставшейся функцией, это не та оставшаяся функция, которая по сути выполняет 10 действий! Я действительно думаю, что такое разрушение функции упрощает тестирование.
mtnpaul
214

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

  1. Глубоко вложенные управляющие структуры : например, циклы for на 3 уровня или даже на 2 уровня с вложенными операторами if со сложными условиями.

  2. Слишком много государственных определяющие параметров : По государственному определяющему параметру , я имею в виду параметр функции , которая гарантирует определенный путь выполнения через функцию. Получите слишком много параметров этого типа, и вы получите комбинаторный взрыв путей выполнения (обычно это происходит в тандеме с №1).

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

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

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

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

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

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

Райан Делукки
источник
1
Хороший пост! Очень детерминированный
Чак Конвей
2
@PedroMorteRolo Совершенно верно. Стандартный API не всегда является образцом элегантности. Более того, большая часть API Java была разработана с глубоким знанием компилятора Java и JVM, поэтому у вас есть соображения производительности, которые могут это объяснить. Я признаю, что критические участки кода, которые не могут тратить впустую ни одной миллисекунды, возможно, должны нарушать некоторые из этих правил, но это всегда следует рассматривать как особый случай. Предварительная трата дополнительного времени на разработку - это начальное вложение, которое может избежать будущего (потенциально наносящего ущерб) технического долга.
Райан Делукки
2
Между прочим .. Я за то, что длинные методы плохо эвристики применимы и к классам. ИМХО, длинные занятия - это плохо, потому что они нарушают принцип единой ответственности. Было бы забавно, если бы компиляторы выдавали предупреждения как для длинных классов, так и для методов ....
Педро Роло
3
@PedroMorteRolo Я определенно согласен с этим. Кроме того, большие классы, вероятно, будут иметь более изменчивое состояние: что приводит к коду, который очень трудно поддерживать.
Райан Делукки
1
Лучший ответ. Еще одна хорошая подсказка: как выглядят комментарии в коде? Количество раз я наткнулся на чей - то код с линии , как: // fetch Foo's credentials where Bar is "uncomplete". Это почти наверняка имя функции, и его следует отделить. Наверное, хочет, чтобы его реорганизовали во что-то вроде: Foo.fetchCredentialWhereBarUncomplete()
Джей Эдвардс
28

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

jmucchiello
источник
18

Функция должна делать только одно. Если вы делаете много мелких вещей в функции, сделайте каждую мелочь функцией и вызывайте эти функции из длинной функции.

Чего вы действительно не хотите делать, так это копировать и вставлять каждые 10 строк вашей длинной функции в короткие функции (как предлагает ваш пример).

Джереми Рутен
источник
Да, создавать множество мелких функций с помощью шаблона копирования и вставки - не
«сделать что-то одно» может быть правильным, а может и не быть, в зависимости от степени детализации. Если функция умножает матрицу, ничего страшного. Если функция строит виртуальную машину - это «одно», но это еще и очень большое дело. Для создания автомобиля компонент за компонентом можно использовать несколько функций.
void.pointer 09
16

Я согласен, что функция должна делать только одно, но на каком уровне это одно.

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

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

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

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

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

bruceatk
источник
2
+1 «Используемая метрика - это функциональность, а не строки кода»
Коди Пирсолл
Еще одна важная метрика - количество уровней вложенности блоков. Сведите к минимуму. Часто помогает разделение функции на более мелкие части. Другие вещи тоже могут помочь, например, многократный возврат.
user2367418
10

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

Почему я могу разбить функцию:

  • Это очень долго
  • Это делает код более удобным в обслуживании, разбивая его и используя осмысленные имена для новой функции.
  • Функция не связная
  • Части функции полезны сами по себе.
  • Когда сложно придумать осмысленное имя для функции (вероятно, она делает слишком много)
dajorad
источник
3
Хорошие моменты, я согласен, также, если вам нужно назвать функцию DoThisAndThisAndAlsoThis, вероятно, она делает слишком много. спасибо :)
2
Вы просто не в порядке с этим приятелем. 60 строк всегда будет слишком много. Я бы сказал, что если вы приближаетесь к 10 линиям, вы, вероятно, близки к пределу.
willcodejavaforfood
Но другая функция все еще вызывает эти функции и, по сути, та же самая DoThisAndThisAndAlsoThisфункция, но с большим количеством абстракций, которые вам все еще нужно как-то назвать,
Тимо Хуовинен
6

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

Ферруччо
источник
4
... при этом установили размер шрифта на 5?
EricSchaefer,
5

Размер примерно соответствует размеру экрана (так что возьмите большой широкоэкранный поворотный экран и поверните его) ... :-)

Шутка в сторону, одна логическая вещь на функцию.

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

/ Johan

Johan
источник
Хорошее замечание о модульном тестировании :)
5

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

function build_address_list_for_zip($zip) {

    $query = "SELECT * FROM ADDRESS WHERE zip = $zip";
    $results = perform_query($query);
    $addresses = array();
    while ($address = fetch_query_result($results)) {
        $addresses[] = $address;
    }

    // now create a nice looking list of
    // addresses for the user
    return $html_content;
}

намного лучше:

function fetch_addresses_for_zip($zip) {
    $query = "SELECT * FROM ADDRESS WHERE zip = $zip";
    $results = perform_query($query);
    $addresses = array();
    while ($address = fetch_query_result($results)) {
        $addresses[] = $address;
    }
    return $addresses;
}

function build_address_list_for_zip($zip) {

    $addresses = fetch_addresses_for_zip($zip);

    // now create a nice looking list of
    // addresses for the user
    return $html_content;
}

Такой подход имеет два преимущества:

  1. Когда вам нужно получить адреса для определенного почтового индекса, вы можете использовать легко доступную функцию.

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

[С другой стороны (я буду отрицать, что говорил вам об этом, даже под пытками): если вы много читали об оптимизации PHP, вы могли бы придумать, чтобы количество функций было как можно меньшим, потому что вызов функций очень, очень дорого в PHP. Я не знаю об этом, так как я никогда не делал никаких тестов. В таком случае вам, вероятно, лучше не следовать ни одному из ответов на свой вопрос, если ваше приложение очень "чувствительно к производительности" ;-)]

EricSchaefer
источник
спасибо, хороший пример :) Я буду гуглить тесты функций в php
5

Взгляните на цикломатику МакКейба, в которой он разбивает свой код на граф, где: «Каждый узел в графе соответствует блоку кода в программе, где поток является последовательным, а дуги соответствуют ветвям, взятым в программе. "

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

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

Чтобы определить, какого размера должны быть ваши методы с точки зрения количества блоков на метод, вы можете задать себе один вопрос: сколько методов мне нужно иметь, чтобы минимизировать максимальное потенциальное количество зависимостей (MPE) между всеми блоками?

Этот ответ дается уравнением. Если r - количество методов, которые минимизируют MPE системы, а n - количество блоков в системе, то уравнение выглядит следующим образом: r = sqrt (n)

И можно показать, что это дает количество блоков на метод, также равное sqrt (n).

Эд Кирван
источник
4

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

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

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

оборота Saltmeister
источник
2

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

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

Вы уверены, что там нет кусочков, которые могли бы пригодиться в другом месте? Что это за функция?

Джеффри Хантин
источник
Функция создает файл кеша из шаблона на основе URL-адреса, например post_2009_01_01.html из URL-адреса / post /
2

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

Pierpaolo
источник
2

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

Олаф Кок
источник
Мне нравится комментировать свой код, в основном не для меня, а для других, что устраняет множество вопросов о том, где была определена переменная $, но мне также нравится, чтобы код был понятным. Врут ли комментарии?
да, потому что в большинстве случаев они не поддерживаются. На момент написания они могли быть правильными, но после исправления ошибки или введения новой функции никто не заставляет изменять комментарии в соответствии с новой ситуацией. Названия методов врут гораздо реже, чем комментарии. ИМХО
Олаф Кок,
Я только что наткнулся на этот ответ: stackoverflow.com/questions/406760/…, в котором говорится, что «Большинство комментариев в коде на самом деле являются пагубной формой дублирования кода». Также - там длинная строка комментариев.
Олаф Кок
1

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

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

Я не знаю, есть ли на этот вопрос правильный или неправильный ответ (например, вы можете выбрать 67 строк в качестве максимального, но могут быть моменты, когда имеет смысл добавить еще несколько).

Эндрю Хеджес
источник
Что ж, мне также нравится видеть свою полную функцию на экране :) иногда это означает шрифт Monospace 9 и большое разрешение на черном фоне, я согласен, так легче понять.
1

По этой теме было проведено тщательное исследование, если вы хотите как можно меньше ошибок, ваш код не должен быть слишком длинным. Но он также не должен быть слишком коротким.

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

См. «Оптимальный размер класса для объектно-ориентированного программного обеспечения» для дальнейшего обсуждения.

Ян Хикман
источник
спасибо за ссылку, чтение :)
1

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

По сути, хотя функция составляла 500 строк, независимо поддерживаемые области в среднем составляли 5 строк.

Джошуа
источник
1

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

Если ваш тест достаточно сфокусирован, он заставит вас написать небольшую целенаправленную функцию, чтобы тест прошел.

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

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

lexicalscope
источник
1

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

Каждый цикл for, оператор if и т. Д. Тогда не рассматривается как ветвь в вызывающем методе.

Cobertura для кода Java (и я уверен, что есть другие инструменты для других языков) вычисляет количество if и т. Д. В функции для каждой функции и суммирует их для «средней цикломатической сложности».

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

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

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

Martin54
источник
0

Я подозреваю, что вы найдете на это много ответов.

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

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

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

Phil.Wheeler
источник
0

предполагая, что вы делаете что-то одно , длина будет зависеть от:

  • что ты делаешь
  • какой язык вы используете
  • сколько уровней абстракции нужно обрабатывать в коде

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

оборота Рэй Тайек
источник
Я занимаюсь кешированием в PHP, да, наверное, 60 строк слишком много, рефакторинг ...
0

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

gkrogers
источник
0

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

Марк Бостлман
источник
0

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

sokket
источник