о функциях (программистское)

[avva]

Эта запись будет интересна только программистам.

Предлагаю вопрос о том, как обустраивать код. Представьте себе, что у вас есть функция foo() (неважно, на каком языке; может, это метод, а не функция - неважно), вся работа которой - вызвать какие-то другие четыре функции A(), B(), C(), D(), которые расположены в других файлах и которые и делают всю основную работу. foo() должна приготовить для них аргументы, передать результаты работы A() в B(), и так далее. Логика foo() выглядит очень просто:

1. Вызвать A().
2. Вызвать B().
3. Вызвать C().
4. Если результат, который вернула C, интересный (условие на одну строчку), вызвать D().

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

Есыь предложение разбить функцию foo(), выделив каждый из логических кусков в отдельную функцию - скажем, doA(), doB() итд. - чтобы foo() только их вызывала. Противник этого преедложения говорит, что на данный момент нет никаких оснований считать, что кому-то еще понадобиться вызывать doA(), doB() итд., кроме foo(). Кроме того, тестировать отдельно doA(), doB() итд. тоже не надо - у главных функций A(), B() итд. есть свои тесты, и у foo() будет свой тест. С другой стороны, сторонник этого предложения, соглашаясь с этим, говорит, что все равно foo() слишком длинна, и что раз есть возможность выделить ее части в отдельные функции, правильным будет сделать этот рефакторинг. После него код будет читабельнее, понятнее, и удобнее для поддержки.

Как вы считаете? И какие аргументы выдвинули бы в поддержку своей точки зрения?

Comments

[lazyreader.livejournal.com]

Я бы выделил отдельные функции и вызывал бы их.

1. Наглядность.

2. Уменьшается сцепление ("coupling"). Кроме того, пространства имён функций doA(), doB()... явным образом сделаны независимыми.

3. Явно ограничивается время существования ненужных временных объектов, которые понадобилось создавать для выполнения doA(), doB()... Ну и стековый фрейм экономится заодно.

3.

[secondary-tea.livejournal.com]

3. блоки {} делают то же самое, а стековый фрейм при разделении понадобится глубже как минимум на один вызов.

[psi-storm.livejournal.com]

Для меня главным критерием выделения какого то кода в функцию является необходимость вызывать этот код из разных мест, то есть я на стороне противника. И 40 строк это нормальный размер, помещается на экране, будет 80 надо будет уже думать =)

[vodianoj.livejournal.com]

Хороший вопрос. Я постоянно с этим сталкиваюсь в Джаве - там у кода есть тенденция обростать.
Я сам в таких случаях не разбиваю, но не уверен, что это хорошо.
Мне кажется, что для повышения читабельности в таком случае можно воспользоваться комментариями и пробелами, искуственно выделяющими вызов А, В, С и Д.

[0qwerty0.livejournal.com]

+1.

[beldmit.livejournal.com]

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

[lekim.livejournal.com]

Сильно зависит от того как подготавливаются данные, но в общем если:
1. doA-B-C могут где то еще пригодится
2. Обработка данных содержит значимый елемент логики, а не просто перекладывание яиц из листа в массив

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

[shufel.livejournal.com]

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

[krolik-ja.livejournal.com]

+1

[wixus.livejournal.com]

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

[stga.livejournal.com]

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

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

[alexeymitin.livejournal.com]

) согласен
функция - она на то и функция, что-бы выполнять какую-то функцию ;)

единственный вариант когда надо выделять в отдельные функции - если в других местах A() также вызывается именно в виде doA() т.е. с теми же аргументами ну и т.п.

[cmm.livejournal.com]

ежели размножение внешне-видимых сущностей (сиречь подлежащих по бюрократическим соображениям тестированию) нежелательно, то можно оставить одну глобальную функцию, внутре у которой будет сколько надо вспомогательных.  oh wait, this is the year 1965, какие такие неглобальные функции...

"жениться вам надо, барин". :)

[avva.livejournal.com]

Что-то ты не так понял. Бюрократических соображений нет: добавление функций не влечет за собой обязательно их тестирование.

[dinizga.livejournal.com]

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

С другой стороны, если эти 10 строк - это логи и ассерты и это повторяется в миллионе мест - можно подумать об AOP, так было бы изящнее.

[pphantom.livejournal.com]

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

[trueblacker.livejournal.com]

пожалуй, наиболее близкая для меня позиция
так что +1

[(Anonymous)]

Я считаю, что do*() нужны.

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

[silly_sad.livejournal.com]

Если вы не можете лаконично и исчерпывающе на Естественном Языке описать спецификацию функции, значит эта функция не имеет права на существование.

таков критерий.
а то что там длинный или короткий получается код -- это категорически неважно.

важна лишь адекватность реальному миру. Есть естественное деление которое можно отобразить функцией -- выделяем её. всё.

[kill-off.livejournal.com]

Если вы не можете лаконично и исчерпывающе на Естественном Языке описать спецификацию функции, значит эта функция не имеет права на существование.

пожалуй, плюсану.

[getman.livejournal.com]

Не хватает информации для взвешенного ответа.
Но ради одного только размера функции я бы не стал добавлять лишний уровень.

[egorfine.livejournal.com]

"код будет читабельнее, понятнее, и удобнее для поддержки"

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

public function pushMessageAction($message) { 
  $userModel = new Model_User();
  $target = $userModel->findByNickname($message->['nickname']);
  if (!$target) { 
    return false; 
  }
  ...
}

сравним:

public function pushMessageAction($message) { 
  if ($this->incorrectNickname($message)) { 
    return false;
  }
  ...

(пример выдумал тут же)

(да, я знаю что $target и $userModel могут потребоваться еще раз тут же, в pushMessageAction())

(да, выйгрышь - только в читаемости кода)

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

[avva.livejournal.com]

В вашем примере incorrectNickname() выглядит как что-то, что может в принципе быть полезным и другим функциям, и не привязано по логике к pushMessageAction(). А что если это не так?

)

[sk-mobile.livejournal.com]

entia non sunt multiplicanda praeter necessitatem

Re: )

[dimrub.livejournal.com]

Да, этот аргумент тоже был выдвинут :)

[old-radist.livejournal.com]

Вопрос не технический, но политический. Нужно смотреть, как проект устроен. Если есть вероятность, что кто-то начнет использовать doA(), doB() etc. не зная, что они сделаны на самом деле для foo(), а потом возьмется их фиксить под себя, не спросив старших товарищей, то выделять функции не надо.

Если такой опасносте нет, то не все ли равно, как делать? Пусть люди самовыражаются, как хотят.

[(Anonymous)]

В паскале для этих целей есть вложенные функции. Однако, Си такой фичей обделили.

[trurle.livejournal.com]

Если функция длинной в 40 строк кажется слишком тяжелой, то так недолго и до мышей.

[vladfrost.livejournal.com]

Вот, отличный комментарий. Всегда имеет смысл выделить часть кода в отдельный метод и дать ему говорящее название, даже если часть кода - это всего одна или несколько строк. Таким образом можно легко делать код самокомментируемым. Об этом ещё старина Фаулер писал (http://www.books.ru/shop/books/30436).

[meshko.livejournal.com]

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

[meshko.livejournal.com]

О, вот и аргументы пришли в голову:

1) код возврата скорее всего придется проверять в двух местах? И в doA() и потом, после вызова doA().
2) Что будет, когда добавится функция, которая вызывает те же функции, но по-другому? С другими аргументами? Будет добавлять doA2(), doB2()? Сделаем doAEx(), которая умеет вызывать A по-разному? Ради чего?

Не разбить

[(Anonymous)]

Если doX() будут вызываться во многих местах - разбить, иначе лишено смысла.

Читабельность 4-х и 40-ка строк неотличимы. Сам практикую "послойное" обустраивание. X() и foo() четко ложатся в два последовательных слоя, doX() - ни туда ни сюда. Если все же doX() нужны, до они должны маскировать X(), т.е. с уровня где лежит foo() должны быть видны только они.

[http://users.livejournal.com/zubr_/]

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

[igorlord.livejournal.com]

There is only one criteria usually -- abstraction, not the length of a function.

If doA(), doB(), ... represent good abstractions in themselves, they should be made so. If they are a cludge that cut across multiple abstractions, no way!


(There are very rare exceptions, like when where is a "nasty but necessary hack", it should live in a function of its own, with a fat comment, despite of the abstraction barrier.)

[tlkh.livejournal.com]

Я бы оставил все как есть и занялся бы чем-то более полезным.

[nevsky.livejournal.com]

Я бы не разбивал. Аргументы все уже озвучены выше (40 строк и почти линейная логика - это реально мало, незачем умножать сущности без нужды; многократные проверки результата; читаемость уменьшается за счет разнесения кода; если doX() не встраиваемые, то теряется времени на вызовы)