Comments 195
DRY и маленькие функции — это не обязательно плохо (даже если это следует из лукавого заголовка). Но они вовсе не обязательно хороши.
Давайте так, вся проблема с определением "дублирование" и "маленькое". В целом вся статья сводится к принципу единой ответственности который хоть и звучит очень просто на поверху оказывается далеко не столь очевидным.
Если рассматривать код через призму потока изменений требований, все становится намного проще. Два куска кода которые выглядят одинаково но имеют под собой двух разных стэкхолдеров, которые могут генерировать изменения независимо друг от друга, как бы говорят что несмотря на похожесть это все же разный функционал и не надо его DRY-ить.
Так же и по поводу "маленькое". Вопрос в том сколько причин для изменений. У вас может быть кусок кода в 1000 строк у которого все еще только одна причина для изменений.
Так же можно попробовать углубиться в Responsibility Driven Design. Там много приемов для выделения ролей и зон ответственности кода.
Была бы у меня возможность поставить +1 поставил бы за хорошую мысль.
Вкусовщина какая то. Не нравится имена функций читать ей.
Основания для выделения куска кода в отдельную функцию — реализуется логически законченный процесс И данный код будет использован повторно. Оба условия обязательны. Если это одноразовый код, то выделять его в отдельную функцию можно временно для целей тестирования.
Ну и функции из одной строки это очевидный бред, до такого маразма конечно не стои опускаться.
Оба условия обязательны.
То есть вы никогда кусок кода в приватный метод/функцию в контексте модуля не выносили для увеличения читабельности и снижения когнитивной нагрузки на читателя?
То что вы обозначили может быть причиной для введения новой абстракции, а функции дробить в пределах модуля — как считаете удобнее так и делайте. Главное что бы это было кохизив и со связанностью проблем небыло.
Для повышения чтабельности достаточно просто написать комментарий к коду. Это и полезнее будет.
Это и полезнее будет.
Пищеварение улучшается и шерстка лоснится?
Его видит только человек, а не компилятор и не IDE.
Его актуальность очень легко утрачивается.
Неактуальный комментарий намного хуже отсутствующего.
Компилятору на читабельность вашего кода вообще фиолетово. Можете хоть спагетти код писать. Если он будет без ашипок, компилятор это вролне устроит.
- Актуальность имён функций утрачивается так же легко, и они так же легко врут.
- Комментарий может отсутствовать, а имя функции не может.
- Комментарии в длине не ограничены, а от длины имени функции зависит удобство её использования. Но чем короче имя функции, тем больше шансов, что оно что-то утаивает.
а от длины имени функции зависит удобство её использования.
если ваша функция слишком длинная и скажем имеет в своем названии And
это признак того что оно делает слишком много.
Докблоки перед функцией никто к слову не отвечал если функция покрывает какую-то сложную концепцию или вычисления нуждающиеся в разъяснениях.
Идея "дробления" ближе к open/close принципу — методы должны соблюдать SRP просто потому что тогда больше шансов что вместо изменения имеющейся функции мы создадим новую. При этом у нас будет новое название функции.
В целом комментарии как правило создают лишнюю когнитивную нагрузку. Но не всегда конечно.
если ваша функция слишком длинная и скажем имеет в своем названии And это признак того что оно делает слишком много.
Ну делает и делает, это не повод её дробить. Есть другие причины для дробления ― пожалуйста. Упрощение тестирования, например. Нет других причин ― оставьте бедную функцию в покое.
В целом комментарии как правило создают лишнюю когнитивную нагрузку. Но не всегда конечно.
Я лишние функции не создают нагрузку разве?
Вот есть операция из пяти стадий, которые выполняются друг за другом и друг без друга не работают. Бьёшь большую функцию на пять маленьких и одну управляющую и получаешь в исходнике шесть артефактов вместо одного. Поди потом помни и объясняй всем, зачем они нужны и в каком порядке их правильно вызывать. Как бонус, в большой функции её интимные локальные данные были скрыты от посторонних взглядов, а теперь эти детали реализации торчат наружу в виде аргументов.
Только не надо говорить, что это не проблема. В C# для её решения сделали локальные функции.
Ну делает и делает, это не повод её дробить.
Не повод конечно, это скорее как кандидат на дробление, но дробить, как вы верно подметили, нужно когда в этом виден смысл.
Я лишние функции не создают нагрузку разве?
если у вас блок с кодом и там есть пара строк которые вы хотите отдельно отметить комментарием — мы будем читать все-равно все эти три строчки. Далее чем больше блок который мы хотим отдельно выделить в контексте кода комментарием, тем больше вероятность что нам будет выгоднее вынести весь этот блок как отдельную функцию, тем самым устранив необходимость фильтровать информацию читателю.
Поди потом помни и объясняй всем, зачем они нужны и в каком порядке их правильно вызывать.
у вас будет одна публичная функция и 5 приватных. Потому знания о том что в каком порядке будет вызываться будут сокрыты.
Только не надо говорить, что это не проблема. В C# для её решения сделали локальные функции.
Это по сути и есть "приватные" функции. Можно так же объявить замыкания и дать пояснение тому что они делают при помощи переменной.
Далее чем больше блок который мы хотим отдельно выделить в контексте кода комментарием, тем больше вероятность что нам будет выгоднее вынести весь этот блок как отдельную функцию, тем самым устранив необходимость фильтровать информацию читателю.
В результате линейная последовательность операций получается записанной в исходнике нелинейно. И читателю (в т.ч. нам в будущем) приходится прыгать взад-вперёд по исходнику, чтобы восстановить в голове полную картину происходящего.
Если есть другие причины бить большую функцию на мелкие, кроме её размера, ― бей; не других причин ― оставь как есть, не порть.
у вас будет одна публичная функция и 5 приватных.
Или была одна приватная, стало шесть приватных, одинаковых с виду. Сразу весело.
Потому знания о том что в каком порядке будет вызываться будут сокрыты.
Не понял. Если я читаю исходник, мне не надо, чтобы логика работы была скрыта, мне надо, чтобы всё было кристально прозрачно. Если же я простой потребитель публичного API, то мне пофиг на исходник и как оно там внутри написано.
Это по сути и есть "приватные" функции. Можно так же объявить замыкания и дать пояснение тому что они делают при помощи переменной.
Вероятно, имелись в виду лямбда-функции. Но в любом случае ты понимаешь, что некоторая проблема есть, и чрезмерное количество мелких функций небесплатно с точки зрения поддержки.
В результате линейная последовательность операций получается записанной в исходнике нелинейно.
Не совсем верно. У вас выходит разделение большой задачи на задачи поменьше. Есть общее описание задачи, где все разделено на этапы, и есть отдельные этапы. Те могут дробиться так же, в зависимости от того нужно это или нет.
Я не знаю. может быть мы с вами по разному код читаем, но я предпочитаю идти "сверху вниз" а не пытаться сначала прочитать все исходники на самом мелком уровне а уже потом пытаться воспроизвести в голове картину.
Или была одна приватная, стало шесть приватных, одинаковых с виду. Сразу весело.
что значит "одинаковых свиду"?
Не понял. Если я читаю исходник, мне не надо, чтобы логика работы была скрыта, мне надо, чтобы всё было кристально прозрачно.
Предположим у нас есть юзкейс, который затрагивает довольно много всего. Вы бы хотели прочитать все исходники относящиеся к этому юзкейсу или хотели бы, что бы весь сценарий был описан в одном месте с которого можно было бы спуститься чуть ниже с точки зрения детализации?
Если же я простой потребитель публичного API
Просто думайте так, как если бы вы всегда были просто потребителем публичного API. Даже когда это API разрабатываете вы.
Вероятно, имелись в виду лямбда-функции.
Вот есть операция из пяти стадий, которые выполняются друг за другом и друг без друга не работают.
Бьёшь большую функцию на пять маленьких и одну управляющую и получаешь в исходнике шесть артефактов вместо одного.
6 артефактов по 15 строк лучше одного из 90.
Поди потом помни и объясняй всем, зачем они нужны и в каком порядке их правильно вызывать.
Зачем объяснять? Параметры функций, их возвращаемые результаты и тело управляющей функции все скажут за вас.
Более того, все пять стадий и зависимости между ними станут явными и будут выражены непосредственно в коде, а не в ваших комментариях.
Как бонус, в большой функции её интимные локальные данные были скрыты от посторонних взглядов, а теперь эти детали реализации торчат наружу в виде аргументов.
Как будто что-то плохое.
Управляющая функция имеет ту же сигнатуру, что и исходная — ее детали наружу не торчат.
Аргументы стадийных функций отражают данные, необходимые для каждого этапа большого процесс.
И как бонус — вы можете переиспользовать стадийные функции в другом варианте большого процесса.
Только не надо говорить, что это не проблема. В C# для её решения сделали локальные функции.
Локальные функции шарпа сделаны совсем по другой причине. Это всего лишь сахар для тех случаев, когда лямбды не очень удобны (рекурсия и т.п.).
На практике, одна функция на 90 строк часто читается в несколько раз быстрее, чем 6 функций по 15.
Эти ф-и по 15 строк могут просто не выполнять какого-либо действия, которое было бы осмысленным вне общего алгоритма, в итоге вам точно так же надо будет выстроить 90-строчную ф-ю, чтобы понять, что именно делает та или иная ф-я по 15. Только сделать это надо будет в уме, на что, конечно, будет потрачено лишнее время.
На практике, одна функция на 90 строк часто читается в несколько раз быстрее, чем 6 функций по 15.
Если функции сделаны прилично, то вместо 90 строк обычно нужно прочесть одну из 6 и потом еще 15 (в случае, если нужны детали реализации), а то и верхнего уровня достаточно бывает. Даже не знаю, что быстрее прочесть 90 строк или 21?
Эти ф-и по 15 строк могут просто не выполнять какого-либо действия, которое было бы осмысленным вне общего алгоритма
Строки которые не содержат никакого законченного осмысленного действия, не нужно выносить в функцию, это написано в любой книге, где может упоминаться негативное влияние размера на удобство чтения и изменения. Только эта статья почему-то размышляет о функциях в контексте их размера, хотя как тут уже много раз замечали, размер лишь симптом плохо спроектированной функции, но не всегда является причиной для дробления. Опыт подсказывает, что для сложной логики бывает, что последовательность вполне равноправных действий может занимать существенное количество строк. Речь скорее о ста, чем о тысячи, конечно, второй случай с почти сто процентной вероятностью это плохой дизайн или последствия оптимизаций (этот случай обычно отмечается специальными комментариями, чтобы не было соблазна «привести в порядок», но случай очень редкий).
Но многие, между тем, так делают, бездумно следуя неким принципам.
> Только эта статья почему-то размышляет о функциях в контексте их размера, хотя как тут уже много раз замечали, размер лишь симптом плохо спроектированной функции, но не всегда является причиной для дробления.
Статья размышляет не о функциях, статья размышляет о программистах, которые часто подвержены карго-культу и делают действия, смысла которых не понимают. Сказали человеку: «дроби ф-и так, чтобы не больше 10 строк в каждой» — и будет дробить, даже тогда, когда код из-за этого становится нечитаемым. А сделаешь замечание — то сошлется на то, что: «так Х сказал в книжке Y, и вообще это хорошая практика, везде так написано». Вы правда ни разу не встречались с такой ситуацией, когда люди выделяют ф-и _исключительно_ по той причине, что «чтобы не было длинно»?
Но многие, между тем, так делают, бездумно следуя неким принципам.
Принципы на то и придуманы что бы ими руководствоваться осмысленно. А то что вы описываете это больше похоже на культ карго и да, это очень большая проблема увы. Собственно вы ниже об этом упомянули.
На практике, одна функция на 90 строк часто читается в несколько раз быстрее, чем 6 функций по 15.
без конкретных примеров это даже обсуждать нет смысла.
Эти ф-и по 15 строк могут просто не выполнять какого-либо действия, которое было бы осмысленным вне общего алгоритма
тогда чем вы руководствовались выделяя функции именно таким образом?
На практике, одна функция на 90 строк часто читается в несколько раз быстрее, чем 6 функций по 15.
В вашей практике — верю. В моей таких случаев не встречалось.
Эти ф-и по 15 строк могут просто не выполнять какого-либо действия, которое было бы осмысленным вне общего алгоритма
Заранее об этом точно не известно. Априорный прогноз с высокой вероятностью будет ошибочным.
в итоге вам точно так же надо будет выстроить 90-строчную ф-ю, чтобы понять, что именно делает та или иная ф-я по 15.
Мне этого не потребуется. Сигнатуры функций дадут понять, зачем написана каждая из них. Общая структура будет ясно читаться из тела управляющей функции. И только для выяснения деталей реализации той и иной стадии мне потребуется лезть глубже.
В вашем комментарии есть одно неявное предположение: тело функции читается лучше ее вызова (имя-параметры-результат).
Между тем, критерий выделения блока кода в функцию прямо противоположный: это есть смысл делать тогда и только тогда, когда вызов функции читается лучше ее тела.
И этот критерий точно выполняется для алгоритма, который уже разбит на пять стадий с явными связями между ними.
Никакие ограничения на размер функции и число артефактов в критерии не фигурируют, следовательно "слишком маленьких" функций не бывает, как и "слишком много" артефактов.
когда вызов функции читается лучше ее тела
Так вопрос не в том чтобы прочитать, а в том, чтобы понять алгоритм целиком. Когда в функцию вынесено "подключение к базе данных", это нормально. Это логически законченное действие, которое мы можем вызывать или не вызывать в других местах. А когда в функцию вынесено "первая часть вычисления SHA1", это только увеличивает когнитивную нагрузку, если надо найти почему SHA1 считается неправильно.
Так вопрос не в том чтобы прочитать, а в том, чтобы понять алгоритм целиком.
Сложный алгоритм целиком гораздо проще понять, когда он декомпозирован с назначением имен и явных связей между шагами.
А когда в функцию вынесено "первая часть вычисления SHA1", это только увеличивает когнитивную нагрузку, если надо найти почему SHA1 считается неправильно.
У SHA1 есть точная спецификация. И от разбиения на функции в соответствии с этой спецификацией (включая имена из предметной области алгоритма) когнитивная нагрузка только снизится.
Когда в функцию вынесено "подключение к базе данных", это нормально.
Подключение к базе данных — это другой уровень абстракции, тут уже не просто функцию, а полноценный формальный контракт выделять надо.
Дискуссия, да и сама статья, появилась потому что многие считают, что разделение на маленькие функции всегда делает код понятнее, а это не так.
их специально сделали такими, чтобы они работали быстрее
В чем принципиальная разница с разросшимся методом бизнес-логики? Почему его можно разбить на функции, а заинлайненный для оптимизации код нельзя? Речь ведь не идет о сохранении скорости.
И подскажите, какие оптимизации вы имеете в виду, которые там кардинально на что-то влияют?
Дискуссия, да и сама статья, появилась потому что многие считают, что разделение на маленькие функции всегда делает код понятнее, а это не так.
Статья появилась по старому доброму принципу "кто умеет — делает, кто не умеет — учит".
И декларируется в ней миф о вреде маленьких функций.
На деле же от маленьких функций самих по себе никакого вреда нет, а от больших — есть.
разделение на маленькие функции всегда делает код понятнее
Это ваш тезис, никак не вытекающий из всей предыдущей дискуссии. Более того, выше в этой же ветке, я прямо писал, что "это (выделение функции) есть смысл делать тогда и только тогда, когда вызов функции читается лучше ее тела."
В чем принципиальная разница с разросшимся методом бизнес-логики? Почему его можно разбить на функции, а заинлайненный для оптимизации код нельзя? Речь ведь не идет о сохранении скорости.
Для SHA1 у меня нет никакого желания реверсить код по ссылке. Я уже не раз объяснил почему и что именно я буду с ним делать. И маленьких функций в реализации будет много — спецификация дает формулы и имена для операций всех уровней.
Принципиальная разница в следующем — когда я только написал большой метод, у меня есть понимание как именно он работает, какие зависимости содержит, как лучше их назвать.
Когда есть большой легаси метод, по которому прошло множество правок и оптимизаций, выделение той или иной ответственности в функцию требует реверс-инжиниринга, и, возможно, переписывания с нуля.
Это далеко не бесплатно, поэтому декомпозицию на подзадачи крайне желательно проводить сразу, как только осознается само наличие выделенной ответсвенности, а не когда код уже превратился в спагетти.
Конечно, в программировании конфетку можно сделать из любого легаси, но лечение и здесь многократно дороже профилактики.
На деле же от маленьких функций самих по себе никакого вреда нет
Есть. Реализация размазывается. Нужно делить до некоторого уровня, но не меньше. Уровень определяется логической завершенностью функции, а не числом строк.
разделение на маленькие функции всегда делает код понятнее, а это не так
Это ваш тезис, никак не вытекающий из всей предыдущей дискуссии.
Предыдущая дискуссия началась с категоричного высказывания "6 артефактов по 15 строк лучше одного из 90". Я привел пример, когда это не так. И в других ветках говорил так же. Почему вдруг он стал с ней не связан?
тогда и только тогда, когда вызов функции читается лучше ее тела
Так проблема не в том чтобы прочитать, а в том чтобы понять работу алогритма. У функции может быть емкое понятное название, только оно не поможет понять, почему она работает неправильно.
Я уже не раз объяснил почему и что именно я буду с ним делать.
Вы сказали, что сделаете правильно и понятно, только примера не привели, даже в псевдокоде.
Когда есть большой легаси метод, по которому прошло множество правок и оптимизаций
Нет там таких правок и оптимизаций, которые запутали бы исходный алгоритм. Был тезис, что если есть длинный код на 90 строк, то его лучше разбить на функции по 15 строк. Вот есть код, только разбить его не получается.
у меня есть понимание как именно он работает
Вот, а у другого человека нет. И чтобы понять, ему придется прыгать по всем мелким функциям, собирая все в одну картину.
Есть. Реализация размазывается. Нужно делить до некоторого уровня, но не меньше.
Этот тезис противоречит вот этому:
Уровень определяется логической завершенностью функции, а не числом строк.
Т.е. вы сами же признаете, что логически завершенная функция может быть сколь угодно малого размера. Ограничением является выразительность вызова по сравнению с телом, а не какой-то пороговый минимальный размер.
Следовательно, в маленьких функциям самих по себе ничего плохого нет.
Предыдущая дискуссия началась с категоричного высказывания "6 артефактов по 15 строк лучше одного из 90".
Ничего что высказывание сделано в определенном контексте и является ответом на реплику ниже?
Вот есть операция из пяти стадий, которые выполняются друг за другом и друг без друга не работают.
Бьёшь большую функцию на пять маленьких и одну управляющую и получаешь в исходнике шесть артефактов вместо одного.
Сам автор кода осознает, что его функция делится на пять стадий со связями между ними.
В этом случае преимущество разбиения на шесть меньших функций очевидно.
Вы вычеркнули контекст, добавили квантор всеобщности и получившуюся химеру подвергли критике. Но это ваша химера, я ничего подобного в виду не имел.
Пожалуйста, читайте диалог полностью, ничего не вырезая и не добавляя от себя к высказываниям собеседников.
Этот тезис противоречит вот этому
Т.е. вы сами же признаете, что логически завершенная функция может быть сколь угодно малого размера.
Нет там противоречия. Логическая завершенность может занимать любое количество строк. "До некоторого уровня, но не меньше" с числом строк не связано. Собственно, второе предложение и поясняет, что означает слово "уровень".
Я говорил следующее:
"далеко не всегда мелкие функции приводят к легко понимаемому коду. А по таким комментариям кажется, что всегда."
"Метод isActive() из одной строки вполне нормально выглядит. А скажем такой код уже не очень."
Я спорю с категоричностью высказывания, а не с существованием мелких функций.
Ничего что высказывание сделано в определенном контексте
Сам автор кода осознает, что его функция делится на пять стадий со связями между ними.
Так в том-то и дело, что стадии можно выделить везде. В любой из приведенных реализаций функция sha1_update делится на стадии. Только если разбить, станет более запутанно.
Квантора всеобщности там бы не было, если бы вы или кто-то из собеседников привели конкретный пример. Но они говорят о "часто" или "бывает", допуская оба варианта, а вы о "лучше" и "не встречалось".
Я взял бы спецификацию из RFC3174
И реализовал по функции на каждую операцию.
По вашим ссылкам весьма агрессивная низкоуровневая оптимизация. Там я бы сохранил удобочитаемый код в качестве опорной реализации и оптимизированный для релиза (с комментариями в виде хорошо читаемого эквивалента).
если у нас где-то используется неправильная битовая операция
У вас функции тестами покрыты? Такого рода ошибки обычно хорошо ломают юнит-тесты. SHA1 по спецификации разбит на простые операции, которые очень удобно покрывать тестами.
Ага, "я бы сделал хорошо и понятно")
По вашим ссылкам весьма агрессивная низкоуровневая оптимизация.
Ну и что? Вопрос как раз в обратном, как сделать тот код более понятным. Так что это наоборот подходит для примера.
Такого рода ошибки обычно хорошо ломают юнит-тесты
Ну вот сломался тест на главную функцию sha1(), и что? Скажем, по первой ссылке не будет 2 последних строчек с рекурсией, или плюс вместо минуса в середине. Как узнать, где именно ошибка? Для этого надо понять весь алгоритм, скрытый за абстрактными названиями функций.
Ага, "я бы сделал хорошо и понятно")
Имея RFC сделать "хорошо и понятно" в данном случае несложно (оптимально по производительности не обещаю).
Учтите что на си я сроду не писал.
Ну вот сломался тест на главную функцию sha1(), и что?
А не главные у вас тестами покрыты?
Для этого надо понять весь алгоритм, скрытый за абстрактными названиями функций.
А зачем? Надо открыть RFC (алгоритм там уже есть) и сравнить реализации функций с ним, планомерно покрывая тестами еще не покрытое.
Учтите что на си я сроду не писал.
Так речь не идет о компилируемом варианте. Просто примерно показать, как вынести код в функции. Обсуждение же о рефакторинге кода из 90 строк, а не о написании с нуля.
А не главные у вас тестами покрыты?
А не главные отрабатывают нормально. Я же указал, где ошибка. Какие тесты можно сделать, чтобы ее поймать?
Надо открыть RFC (алгоритм там уже есть) и сравнить реализации функций с ним
А там такие функции и есть, по 30-60 строк, разве что циклы не развернуты. Они не разбиты на стадии.
И да, на бизнес-логику RFC с алгоритмом нет, с чем вы будете сравнивать?
Просто примерно показать, как вынести код в функции.
На сишном коде, оптимизированном до полной нечитаемости?
Для которого уже есть точная спецификация?
Нафига козе баян?
Проще написать самому с нуля или взять готовую реализацию без явных косяков.
По ссылке довольно симпатичная:
https://github.com/git/git/blob/master/block-sha1/sha1.c
В вашей постановке задача не имеет смысла, а слово "просто" надо сразу брать в кавычки: низкоуровневые оптимизации по производительности не имеют ничего общего ни с декомпозицией, ни с читаемостью.
И да, на бизнес-логику RFC с алгоритмом нет, с чем вы будете сравнивать?
А бизнес-логика ничего общего с криптокодом из openssl не имеет.
И да, на сложную бизнес-логику спецификации очень даже пишутся.
По ссылке довольно симпатичная
Код blk_SHA1_Update()
принципиально ничем не отличается от реализации по моей первой ссылке, там тоже 30 строк. Ок, пусть будет эта, как бы вы ее разбили на стадии?
низкоуровневые оптимизации по производительности не имеют ничего общего ни с декомпозицией, ни с читаемостью.
Причем здесь оптимизация? Я же про обратный процесс спрашиваю. Вы говорите, что если есть длинный код, то надо разбить его на мелкие функции, потому что так понятнее. Непонятный оптимизированный код как раз хороший кандидат для применения этого подхода, разве нет? Речь же не идет о том, чтобы сохранить скорость работы.
А бизнес-логика ничего общего с криптокодом из openssl не имеет.
А в чем разница? Есть конкретная реализация вычислений, которая занимает много строк. Вы говорите, что такой код лучше разбивать на маленькие функции для повышения понятности. Я прошу вас показать, а вы начинаете говорить про готовые алгоритмы и переписывание с нуля. То есть такой подход работает не всегда, о чем я и сказал.
И да, на сложную бизнес-логику спецификации очень даже пишутся.
Спецификации пишутся, а готовых функций там нет. Чтобы найти, где код отличается от спецификации, надо понять, что и как он делает. То есть важна не абстракция, а детали реализации.
Причем здесь оптимизация? Я же про обратный процесс спрашиваю.
Декомпозиция — способ реализовать функциональные требования максимально удобным и понятным способом.
Низкоуровневая оптимизация — изменение кода с целью реализации нефункциональных требований с учетом специфики транслятора и среды исполнения.
В коде, подвергнутом такой оптимизации, размер функций и их наличие определяется совсем не соображениями понятности, а только стоимостью с точки зрения производительности (или потребления ресурсов).
Качество такого кода по любому другому критерию может быть ниже всякой критики.
Если у вас сломался оптимизированный на низком уровне код — ищите коммит, где это произошло и разгребайте лапшу.
Непонятный оптимизированный код как раз хороший кандидат для применения этого подхода, разве нет?
Выделение функций в таком спагетти само по себе как мертвому припарки. Так что пример с оптимизированной реализацией SHA1 заслуживает приставки "анти".
Я с таким сталкивался. Иногда самым быстрым способом исправления был реверс алгоритма по коду с последующим переписыванием с нуля. Найти ошибку в исходном варианте было сложнее.
В коде, подвергнутом такой оптимизации, размер функций и их наличие определяется совсем не соображениями понятности, а только стоимостью с точки зрения производительности (или потребления ресурсов).
Но я ведь не прошу написать такой код. Я прошу сделать наоборот — убрать оптимизацию и сделать размер и наличие функций по соображениям понятности. Потому и пример известный выбрал, на который есть описание требований, не писать же полное ТЗ в комментарии.
Выделение функций в таком спагетти
Но ведь это просто код, не разбитый на стадии. Там наоборот нет никакого спагетти из мелких функций. И каких-то больших низкоуровневых оптимизаций нет, за исключением развернутых вычислений по третьей ссылке. На PHP или Java будет примерно то же самое.
Ок, я понял, примера применения подхода не будет.
Там наоборот нет никакого спагетти из мелких функций.
Видите ли, сам термин "спагетти-код" относится как раз к непонятному коду внутри больших функций. "Спагетти из мелких функций" — это ваше изобретение, не знаю как оно выглядит.
Я прошу сделать наоборот — убрать оптимизацию и сделать размер и наличие функций по соображениям понятности.
Вы хотите чтобы за вас оптимизированный код отрефакторил кто-то другой на незнакомом ему языке? И хотите на таком примере делать выводы о пользе маленьких функций?
Это при наличии готовой спецификации и кучи готовых реализаций?
На PHP или Java будет примерно то же самое.
Фортрановскую программу можно написать на любом языке программирования.
Ок, я понял, примера применения подхода не будет.
Лучший путь деоптимизации предложенного вами кода — выкинуть его в мусорку и написать реализацию в лоб по RFC.
Там, кстати, получается множество однострочных функций.
из мелких функций — это равиолли http://wiki.c2.com/?RavioliCode
Видите ли, сам термин "спагетти-код" относится как раз к непонятному коду внутри больших функций.
Тем более, это ведь именно тот случай, который предлагается исправить разбиением на функции.
Вы хотите чтобы за вас оптимизированный код отрефакторил кто-то другой на незнакомом ему языке? И хотите на таком примере делать выводы о пользе маленьких функций?
Вас же не смущает псевдокод или незнакомый язык в учебниках от известных авторов. Этот пример ничем не отличается от того кода, который предлагается разбивать на функции. Есть ТЗ, есть длинный код.
Это при наличии готовой спецификации и кучи готовых реализаций?
Именно потому что есть спецификация и другие реализации. Потому что известны требования и можно сравнить, что понятнее.
Там, кстати, получается множество однострочных функций.
Хорошо, можете привести свой вариант реализации? На любом языке.
Там есть комменты, которые разбивают
1) Эту функцию на на куски
2) Описывают взаимосвязи между кусками типа
/*
- Where do we get the source from? The first 16 iterations get it from
- the input data, the next mix it from the 512-bit array.
*/
Вопрос,
1) почему эти взаимосвязи нельзя выразить явно в коде? Будет ли от этого понятней?
2) Есть ли какое-то предназначение у раундов?
3) Как это все было разработано? Что было в голове у автора и как он взял все эти числа? Нельзя ли это выразить конструкциями языка. Я не спец в крипто — вы можете побыть доменным экспертом?
1) Хм, как бы вы выразили в коде описанное в комменте?
2) Раунд это одна итерация вычисления.
3) Это константы из описания алгоритма.
1) var array = first_16_iterations(inputData)…
2) Чем они друг от друга отличаются? Там есть еще итерации внутри раундов. Есть у них какое-то предназначение?
3) Как появилась эта константа и почему она именно такая?
Т.е. вы понимаете смысл того, что там или фактически выполняете работу компилятора, перенося код из книжки на C и оптимизируя его?
1) Там примерно так и написано
#define T_0_15(t, A, B, C, D, E)
SHA_ROUND(t, SHA_SRC, ...)
#define T_16_19(t, A, B, C, D, E)
SHA_ROUND(t, SHA_MIX, ...)
2) Раунд это понятие предметной области. В SHA_ROUND нет итераций, только битовые операции.
3) Это лучше спросить у разработчиков, придумавших SHA1.
Я никакой код на С не пишу и к приведенным ссылкам отношения не имею.
1) Это не так же — неявно что есть вход что выход
2) С точки зрения авторов кода раунд так же что-то состоящее из итераций.
/ Round 1 — iterations 0-16 take their input from 'block' /
3) То есть мы не понимаем смысла манипуляций там. Как мы тогда можем утверждать, что от разбиения на куски код станет непонятнее? Надо понять алгоритм — что там зачем и почему и тогда можно сделать функции с читаемыми названиями.
1) Предложите свой вариант. Только полный, а не одну абстрактную функцию без тела.
2) Не знаю, почему они так написали, в описании ясно сказано — состоит из 80 раундов.
3) Почему не понимаем. Константы это ТЗ. Неважно в какой функции они будут находиться.
Надо понять алгоритм — что там зачем и почему
И-и вот мы снова к этому пришли. Чтобы что-то изменить в незнакомом алгоритме, надо его понять.
Хороший пример кстати с комментарием. Автор явно использовал слово раунд в смысле отличном от текущего ТЗ, при том что SHA_ROUND названо правильно. Так как это пример, обозначающий некую бизнес-логику, можно предположить вариант, когда в предыдущей версии ТЗ раундом называлась группа из 20 шагов, а потом поменяли. Сильно бы помогло найти несоответствие, если бы вместо комментария было название функции, да еще спрятанное куда-нибудь в performIterations()?
Предложите свой вариант. Только полный, а не одну абстрактную функцию без тела.
Ну вот еще, это надо все это переписывать на язык, в котором можно все это нормально выразить.
Не знаю, почему они так написали, в описании ясно сказано — состоит из 80 раундов.
Зачем они написали комменты вообще? Почему важно, чтобы мы знали что вход что выход.
И-и вот мы снова к этому пришли. Чтобы что-то изменить в незнакомом алгоритме, надо его понять.
А кто говорил, что это не так? Чтобы его понять надо чтобы автор алгоритма передал что и почему он сделал. То есть почему первые N итераций так, а последующе M итераций сяк.
В данном случае мы имеем дело с вырожденным примером — нам передали уже готовый алгоритм, а мы его просто переводим с одного формального языка на другой, не понимая что и зачем — так?
Но тем не менее все равно можно рефакторингом а не комментариями выразить то, что мы о нем знаем. Хотя бы входы и выходы отдельных частей
Ну вот еще, это надо все это переписывать на язык, в котором можно все это нормально выразить.
Хм, подождите, вы говорили, что написано неявно и лучше было написать по-другому. То есть написать по-другому нельзя? В чем тогда претензия к автору кода?
Напишите на другом языке, сравним.
А кто говорил, что это не так?
Был разговор, что много мелких функций дают возможность понимать только некоторые. А понимать надо алгоритм целиком, чему излишнее дробление только мешает.
То есть почему первые N итераций так, а последующе M итераций сяк.
Потому что это написано в ТЗ. Наше дело реализовать, математические доказательства тут неважны.
нам передали уже готовый алгоритм, а мы его просто переводим с одного формального языка на другой, не понимая что и зачем — так?
Нет. Нам передали неизвестный нам алгоритм, который надо понять и предположительно исправить ошибку. Что удобнее понимать — в том виде в котором он есть, или если бы он дополнительно был разбит на много функций?
Мне кажется что первое. Если вам кажется второе, приведите свой вариант разбиения.
А понимать надо алгоритм целиком, чему излишнее дробление только мешает.
Абстракции — это плохо, да?
Там ключевое слово «излишнее».
Что удобнее понимать — в том виде в котором он есть, или если бы он дополнительно был разбит на много функций?
Конечно лучше видеть именованные шаги, вместо единого полотна реализации. Если я хочу понять, как управляется автомобиль, я начну с органов управления, а не полезу под капот. Имена функций — это круче, чем комментарии, а суть та же — документация.
Там ключевое слово «излишнее».
Что значит «излишнее»? Любой пример здесь будет показывать лишь, что программист либо не смог придумать адекватное название, либо нужен более глобальный рефакторинг на тему SRP.
Плохой пример потому, что имена могут и должны опираться на контекст, которого здесь для непосвященного практически нет.
Так в том и смысл, что код незнаком. Вам надо его понять.
И в коде функций по всем 4 ссылкам нет ассемблерных вставок. Они встречаются только в дефайнах для специализированных архитектур, и в одной из веток всегда есть C версия. А магические числа типа 0x5a827999 это часть ТЗ, они будут в любом коде.
Вообще, макросы это как раз те мелкие функции, на которые предлагается разбивать (конечно не технически а логически). Если они вызывают у вас проблемы с пониманием, то это именно то, о чем я говорю. Так ваш код выглядит для "непосвященых".
Так в том и смысл, что код незнаком. Вам надо его понять.
Нет, смысл в том, что я должен отрефакторить длинный, но понятный мне алгоритм.
И в коде функций по всем 4 ссылкам нет ассемблерных вставок.
Вы не туда смотрите
они будут в любом коде.
Сильное заявление. Я предпочел бы заменить их мнемоническими константами.
Сильное заявление. Я предпочел бы заменить их мнемоническими константами.
Там нет мнемонических констант, так как программист просто кодирует алгоритм данный свыше, а автор алгоритма не потрудился объяснить в формальном виде что это за константы и зачем. То есть цель не в том, чтобы понять что-то, а чтобы просто закодировать то, что кто-то другой написал псевдокодом.
Это не код основных функций, на понимание алгоритма он не влияет, и рядом там есть версия на C.
Тем не менее они там будут. И в их названии тоже будут числа — например K1, K2, K3, K4.
Кстати в RFC K это маленькие функции а не числа
K(t) = 5A827999 ( 0 <= t <= 19)
K(t) = 6ED9EBA1 (20 <= t <= 39)
K(t) = 8F1BBCDC (40 <= t <= 59)
K(t) = CA62C1D6 (60 <= t <= 79).
private static (UInt32 A, UInt32 B, UInt32 C, UInt32 D, UInt32 E) DoIntermediateHashCalculationRounds(
uint[] W,
uint A,
uint B,
uint C,
uint D,
uint E)
{
for (var i = 0; i < 80; i++)
{
var (f, K) = GetHashingRoundParameters(i);
var nextA = SHA1CircularShift(5, A) + f(B, C, D) + E + W[i] + K;
E = D;
D = C;
C = SHA1CircularShift(30, B);
B = A;
A = nextA;
}
return (A, B, C, D, E);
}
private static (Func<uint, uint, uint, uint> f, UInt32 K) GetHashingRoundParameters(int roundIndex)
{
var parameterIndex = roundIndex / 20;
Func<uint, uint, uint, uint>[] f = { f0, f1, f2, f3 };
UInt32[] K = { 0x5A827999, 0x6ED9EBA1, 0x8F1BBCDC, 0xCA62C1D6 };
return (f[parameterIndex], K[parameterIndex]);
}
взял вот отсюда
И отрефакторил функцию SHA1ProcessMessageBlock
https://pastebin.com/YRwsBG2M
Мне кажется, так взаимосвязи и что на что влияет понятнее
Оу, спасибо. Скажу личное мнение как незнакомый с кодом, может еще кто напишет.
Инициализацию наверно стоило вынести в функцию, она отличается от обработки. А разбивать не стоило, теперь при изучении одной из них появляется вопрос "wtf, а где другая часть".
10 аргументов в DoIntermediateHashCalculationRounds
намекают что что-то не то. Раньше был четкий цикл на 80 итераций и понятно чему он соответствует в ТЗ, а теперь произвольные startIndex
и firstIndexAfterEnd
, надо держать в голове, что они от 0 до 80 и изменяются по 20, да еще и f разная каждый раз.
Как оно будет работать для 39 бита? Так сходу и не скажешь. Зачем? А если ТЗ поменяется и для него другая функция понадобится, или мы подозреваем ошибку на этом шаге и хотим проверить правильно ли все передается.
В вызывающей функции появился цикл на 4, аналога которого вообще нет в ТЗ, и надо догадаться, что он задает диапазоны для t.
Массовые присвоения тоже снижают понятность, но это ладно, одинаковые названия помогают.
10 аргументов в DoIntermediateHashCalculationRounds намекают что что-то не то.
В принципе, да, ABCD по сути типа массива промежуточных значений. Можно сделать структуру и передавать как целое (надо посмотреть, как это лучше C#)
В вызывающей функции появился цикл на 4, аналога которого вообще нет в ТЗ, и надо догадаться, что он задает диапазоны для t.
Я посмотрел перед этим Rozetta Code там для Ruby такая же штука и мне было понятно :)
То есть написать по-другому нельзя? В чем тогда претензия к автору кода?
Претензий нет. Возможно он сделал все что мог. Но тут мы вроде договорились оптимизации не рассмативать
Я не могу. Насколько я помню С можно по крайней мере декларировать что используется в конкретном куске, а что нет.
Был разговор, что много мелких функций дают возможность понимать только некоторые. А понимать надо алгоритм целиком, чему излишнее дробление только мешает.
По крайней мере, можно понять, что где используется.
Если у нас появится понимаение, что для чего нужно, то мы можем выразить это в понимании функций.
Потому что это написано в ТЗ. Наше дело реализовать, математические доказательства тут неважны.
Если полный алгоритм написан в ТЗ — фактически надо разбивать на функции алгоритм в ТЗ. Чтобы было понятно.
Нет. Нам передали неизвестный нам алгоритм, который надо понять и предположительно исправить ошибку. Что удобнее понимать — в том виде в котором он есть, или если бы он дополнительно был разбит на много функций?
В том виде в котором он есть проще установить соответсвие в ТЗ. Если в ТЗ он правильно написан, то сверкой проще найти ошибку.
Если вдруг у нас появляется понимаение, что там происходит и мы можем дать нормальные названия функциям, то код будет понять проще чем ТЗ.
Мне кажется что первое. Если вам кажется второе, приведите свой вариант разбиения.
Ok. Постараюсь найти что-нибудь на C# и отрефакторить
Зачем объяснять? Параметры функций, их возвращаемые результаты и тело управляющей функции все скажут за вас.
Вижу шесть приватных функций. Нашёл, что одна из них вызывает другие. Ок, с ней понятно.
Остальные пять ― одни делают какую-то общую работу и не имеют смысла друг без друга, или логически независимы и используются или могут использоваться где-то ещё?
Если я удаляю за ненадобностью главную функцию, эти пять можно тоже удалить? IDE могут помочь это выяснить, но не точно и невсегда удобно. Если я таки удалю все функции и ничего не сломается, может всё-таки не стоило удалять ― вдруг был расчёт переиспользовать их функционал? Не зря же их кто-то (или я сам) выделил в отдельные функции.
А если не удалю, получится мёртвый код, который вроде есть, но вроде нигде не используется. Хрен знает, как он тут появился, зачем нужен и что с ним делать.
Более того, все пять стадий и зависимости между ними станут явными и будут выражены непосредственно в коде, а не в ваших комментариях.
Ну и стоило ли ломать зависимости, чтобы потом их восстанавливать? Цепочка действий и так была прекрасно обозначена в исходной функции. Зачем последовательность превращать во множество, чтобы потом из множества опять собирать последовательность? Явные зависимости, если уж есть, завсегда понятнее неявных.
Управляющая функция имеет ту же сигнатуру, что и исходная — ее детали наружу не торчат.
Мы ведь про пишущих/читающих исходники говорим, а не про стороннего пользователя, который видит только публичный API. К такому стороннему пользователю вся эта тема вообще не имеет отношения.
Под "наружу" я имею в виду сигнатуры функций.
Когда есть одна большая функция, у неё все кишки, все детали реализации внутри. Свернул функцию и кишок не видно. Надо изменить логику ― внутри функции изменил, снаружи никто не заменил.
А когда большая операция искусственно раздроблена на мелкие части, изменение внутри одной часто влечёт изменение в других. У одной изменились выходные параметры ― меняй входные у другой. Изменения серьёзнее ― переписывай другие функции целиком. Инкапсуляция близка к нулю.
Аргументы стадийных функций отражают данные, необходимые для каждого этапа большого процесс.
Нет смысла фиксировать детали реализации в сигнатурах функций. Сигнатура ― это контракт, который функций «обещает» выполнить неважно каким способом. И тот контракт видится как нечто более стабильное, чем внутренности. А при искусственном делении оказывается, что не контракт влияет на реализацию, а наоборот.
И как бонус — вы можете переиспользовать стадийные функции в другом варианте большого процесса.
Когда/если это понадобится, тогда можно будет озаботиться. Опыт показывает, что с большой вероятностью не понадобится.
IDE могут помочь это выяснить, но не точно и невсегда удобно.
И точно, и удобно. По крайней мере для языков со статической типизацией.
Цепочка действий и так была прекрасно обозначена в исходной функции.
Неправда. Попробуйте определить как зависит строка 86 большой функции от строки 3. И вот здесь IDE вам скорее всего никак не сможет помочь — придется интерпретировать в голове все тело большой функции.
Нет смысла фиксировать детали реализации в сигнатурах функций.
Гораздо приятнее каждый раз перечитывать тело большой функции при каждом сколь угодно мелком ее изменении.
А при искусственном делении оказывается, что не контракт влияет на реализацию, а наоборот.
Отсутствие навыка декомпозиции — не повод от нее отказываться.
Опыт показывает, что с большой вероятностью не понадобится.
Конечно не понадобится: из тела большой функции после многих правок что-то выделить и переиспользовать сложнее, чем задублировать функциональность по месту.
И точно, и удобно. По крайней мере для языков со статической типизацией.
Удобно ― это когда присутствует CodeLens. Точно ― это когда отсутствует рефлексия. В C# никогда не приходилось расставлять атрибуты [UsedImplicitly]? Как-то перестаёшь после этого доверять показаниям IDE. Умом-то понимаешь, что всё, скорее всего, нормально, но вероятность, что после удаления «ненужного» где-то что-то нае*нётся, есть всегда. Причём эта низкая вероятность компенсируется неочевидностью причины и тяжестью последствий.
По поводу остального: Не надо думать, что если функция большая, то у неё внутри лапша. Забористую лапшу можно лепить и из мелких. Лапша ― проблема, размер ― не проблема и даже не источник.
Если я удаляю за ненадобностью главную функцию, эти пять можно тоже удалить? IDE могут помочь это выяснить, но не точно и невсегда удобно.
Можно безо всякого IDE, если языке поддерживает вложенные функции
private static string GetText(string path, string filename)
{
var sr = File.OpenText(AppendPathSeparator(path) + filename);
var text = sr.ReadToEnd();
return text;
// Declare a local function.
string AppendPathSeparator(string filepath)
{
if (! filepath.EndsWith(@"\"))
filepath += @"\";
return filepath;
}
Ещё они спасают от мусора в Intellisense и красиво решают одну проблемку с лямбдами. Остаётся проблема нелинейности: код читается не как простой текст сверху вниз, а прыжками туда-сюда.
Вот есть операция из пяти стадий, которые выполняются друг за другом и друг без друга не работают.
Если вам о ней удобно думать, как о разбитой на пять стадий, а не на 90, почему бы это разбиение не выразить явно в коде?
Ну да, пустой строкой и комментарием с названием следующей стадии.
Почему не объяснить компилятору, дебаггеру и IDE, что это отдельные стадии, какие переменные используются только внутри стадии, а какие используются для передачи значения между ними? Дебаггер и стектрейс покажет вам на какой стадии вы находитесь, компилятор проконтроллирует, что вы случайно не переиспользовали переменную не предназначенную для этого.
И профайлер вам покажет, сколько какая стадия жрет ресурсов
Почему не объяснить компилятору, дебаггеру и IDE, что это отдельные стадии,
Им эта информация не нужна. Отладчик, профайлер и стектрейсы всё могут показывать с точностью до номера строки ― так оно намного интереснее. Номер строки точно указывает, где проблема, а остальной стектрейс лишь объясняет, каким путём мы сюда попали.
Но повторюсь, если есть другие причины разбивать большую функцию, кроме её размера, то можно и разбить. Нет причин ― не надо бить, это небесплатно.
Отладчик, профайлер и стектрейсы всё могут показывать с точностью до номера строки ― так оно намного интереснее.
Вот вы видите в стектрейсе, что ошибка произошла не строке 666 — это понятнее, чем если ошибка произойдет в строке 666, на стадии 1?
Зачем вы вообще для себя делите на стадии этот код, если строки понятны?
Когда есть номер строки и стектрейс, больше ничего не надо. Всё равно в исходник лезть, а там всё описано.
То есть названия функций в стектрейсах абсолютно бесполезны?
По большому счёту да. Можно, наверное, придумать пример, где это будет важно, но обычно пользы нет.
Когда у меня на руках падаёт чужая программа, показывая только стектрейс без внятного описания проблемы человеческим языком, то по названиям функций изредка можно примерно догадаться, что пошло не так. Сделать что-то практически полезное с этой информацией получается ещё реже.
А стектрейс от моей программы мне интересен только тем, что если в функцию, где произошла ошибка, можно попасть несколькими путями, то стектрейс показывает, который из них привёл к ошибке. Чтобы исправить ошибку, я лезу в исходники, и тут, имея имя файла и номер строки, на имя функции даже не смотрю.
но обычно пользы нет.
связано ли это с качеством выбранных названий? Во всяком случае из приведенного вами же примера с чужой программой я могу сделать ровно такой вывод.
Да никак не связано. Чужая логика работы и чужая внутренняя терминология не обязана совпадать с тем, что думает, видит или ожидает пользователь. Пользователю нужен текст ошибки и описание решения на человеческом языке или какой-нибудь идентификатор ошибки, чтобы по нему загуглить. Стектрейс можно использовать как такой идентификатор. Вот собственно и вся его практическая польза.
Самое полезное в чужих ошибках не стектрейсы, а названия исключений, если есть. Т.е. что случилось, а не где случилось. Если там какой NullReferenceException, то хз, но если FileNotFoundException, то можно догадаться, что чего-то программе не хватает, и ProcMon'ом даже узнать, чего именно.
Drag and Drop не может быть одной функцией просто потому что это onDrag
+ onDrop
. В простых случаях можно обойтись только последним, но тут в дело вступает separation of concerns. Эта функция должна попросить кого-то что-то сделать со стэйтом а не пытаться это все сделать самостоятельно. и вот у вас уже разделение.
Дополню. Естественно программы состоят из множества функций и вовсе не обязательно каждая из них используется повторно. Мой комментарий относится к "мелким, маленьким" функциям, нижнему уровню абстракции, что бы не скатиться к маразму однострочных функций.
давайте так, вы пишите в коде else
? Как часто?
p.s. как размер функции относится к уровню абстракции?
Давайте так, мы все еще обсуждаем статью или вы уже о чем то другом хотите поговорить?
Вопрос стоит ребром, стоит ли вот эти несколько строк выделять в отдельную функцию или нет? Чем мы должны руководствоваться? По мне так, что бы не скатиться ни в одну из крайностей мало больших функций/однострочные функции достаточно двух правил, которые, я озвучил выше. Мало функций так же плохо, как и слишком много.
мы все еще обсуждаем статью или вы уже о чем то другом хотите поговорить?
мы обсуждаем ваше категоричное:
Основания для выделения куска кода в отдельную функцию — реализуется логически законченный процесс И данный код будет использован повторно. Оба условия обязательны.
Чем мы должны руководствоваться?
Здравым смыслом, разумеется. В целом цели которые мы приследуем выделяя какой-то код в отдельные функции:
- упрощение кода для восприятия, потому что код чаще читают чем пишут
- устранение деталей, дабы можно было быстро глянуть общий флоу и если нас интересует конкретная деталь флоу — уже погружаться внутрь.
- самодокументируемый код
достаточно двух правил, которые, я озвучил выше.
и мы придем к следующей проблеме:
- интерпретация "использование повторно"
- логически законченный процесс
Мало функций так же плохо, как и слишком много.
High cohesion, low coupling. Основы структурного дизайна. 45 лет уже прошло. А мы все еще оперируем понятиями "маленький", "большой", "много", "мало", "логически законченный"...
p.s. ответьте на вопрос. Используете ли вы if в коде, существуют ли у вас "большие" блоки условий, или вы выделяете этот код в отдельные функции?
Используется повторно, как и логически законченный это однозначность, а вот упрощение для восприятия это как раз интерпритация. Каждый будет вертеть эту упрощение в свою сторону.
If использую. Блоки выделяю по необходимости, без фанатизма.
Ну так 45 лет не для всех прошло :).
Я руководствуюсь принципами выделения кода в функцию:
1. Если код решает задачу, которую нужно решить еще в другом месте программы. Важно отметить, что это не следует из одинаковости кода! Код может быть одинаков, но решать разные задачи, в этом случае его не стоит выносить в отдельную функцию.
2. Уровень абстракции. Если для достижения уровня абстракции, на котором написан код требуется разделить его на функции, то это необходимо сделать. Обычно уровень абстракции на уровне модуля не меняется (если нужно поменять, значит стоит писать несколько модулей и объединять их в пакет).
Довольно часто это просто следствие написания алгоритма в псевдокоде:
1. если чайник не пустой -> вылить_чайник
2. налить_чайник
3. поставить_чайник_на_плиту
4. включить_плиту
5. если чайник кипит -> выключить_плиту
Я использую if-elif-else. И вызываю по условиям функции самой разной длины.
Причём повторное использование вовсе необязательно. Скажем, у меня есть функции первичной конфигурации — создать базу данных (дефолтнейм), создать к ней пользователя(дефолтюзер), заполнить базу болванкой(дефолтдамп). Это выполняется единственный раз. Дальнейшая работа программы даже после сотого перезапуска уже не обратится к этим функциям.
Насколько плох такой подход?
И вызываю по условиям функции самой разной длины.
ну то есть вы блоки с кодом все же выделяете в отдельные функции что бы можно было просто блок с условиями прочитать как сценарий, "если это задано, то надо делать то-то, иначе делать то-то". Это я и имел ввиду, когда спрашивал про else
всякие. В этом случае одно из условий "выделения функции" озвученное выше не соблюдается (либо мы имеем дело с конфликтом терминологии). но при этом нам удобнее.
Насколько плох такой подход?
Ну если вам удобно, то видимо не плохо. Плюсы минусы — вам находить.
def create_db():
some spam
def create_user():
some eggs
def dump_load():
take spam with eggs
insert them into base
while (stopflag == False):
if (%no_base%):
create_db()
if (%no_user%):
create_user()
if (%no_tables%):
dump_load()
main_logic_continues
Не лучше ли развернуть описанное в одну линию?
Вот главный вопрос.
https://martinfowler.com/bliki/FunctionLength.html
Once I accepted this principle, I developed a habit of writing very small functions — typically only a few lines long [2]. Any function more than half-a-dozen lines of code starts to smell to me, and it's not unusual for me to have functions that are a single line of code [3]. The fact that size isn't important was brought home to me by an example that Kent Beck showed me from the original Smalltalk system. Smalltalk in those days ran on black-and-white systems. If you wanted to highlight some text or graphics, you would reverse the video. Smalltalk's graphics class had a method for this called 'highlight', whose implementation was just a call to the method 'reverse' [4]. The name of the method was longer than its implementation — but that didn't matter because there was a big distance between the intention of the code and its implementation.
Основания для выделения куска кода в отдельную функцию — реализуется логически законченный процесс И данный код будет использован повторно. Оба условия обязательны.Все-таки не соглашусь. Читая код функции я ожидаю, что каждая ее строка будет одного уровня абстракции с другими. И если какой-то одноразовый код выбивается из контекста этой ф-ции, выпуская кишки (нижние абстракции) наружу, лучше дать ему осмысленное имя, выделив в отдельную функцию, согласно контексту вызывающей стороны. И не важно один раз будет вызвана эта функция или нет.
Сейчас они нижние, а через час — херакс, верхние. Потому, что *надо*! Прикинь… Потому, что практика, инженерия, мать-её, а не филология и розовые пони.
@#$@ филологи-говнометарии даже в комитет по стандартизации С++ пролезли, и придумали private, protected и const. Ну и, мать-их, ссылки, ради детей…
Потому, что практика, инженерия, мать-её, а не филология и розовые пони.
Ну вот смотри, Петрович, мы стены поставили, перекрытие сделали, а потом вспомнили что нужен фундамент! Ну короч мы сверху жахнули все это дело, не пропадать же. Инженерия, едрить на лево!
Да. И после таких как вы в коде разобраться без поллитра невозможно.
К вопросу коротких функций — всё хорошо в меру. Ничего плохого в цикле на два-три экрана с индентами на 10 позиций я не вижу, если это оправдано. Также ничего плохого нет в маленьких функциях, но тут уже работает филология.
Практическая часть… В одном проекте на Эрланге я встречал маленькие функции do1, do2, do3, do4, do5… Это было разделение одной большой функции do на маленькие с паттерн-матчингом. Так лучше-бы эти @#$@ написали одну большую функцию.
В проекте на С++, который разрабатывался лет 10 я встретил несколько маленьких функций convertLogicalToPhysical, convertPhysicalToLogical. Эти термины не употреблялись херову тучу лет и никто не знал что они означают, потому, что архитектура поменялась неузнаваемо. Я потратил часа 4 на то, чтобы понять, что за херня там написана, потом плюнул и написал длинный цикл с аналогами этих дебильных функций.
Такие дела.
Это не «хук-хук и в продакшен», а практика.
у нас с вами может быть разная практика. Вы описываете практику где недостает рефакторинга. Ну то есть не того рефакторинга когда все к чертям сносится и пилится с нуля, а когда "узнали как это лучше назвать — переименовали".
Словом мне остается непонятна эмоциональная подоплека вашего комментария.
Неясность возникает тогда, когда нужно определить «что-то одно».
Достаточно только понимать и принимать одну вещь: "что-то одно" может быть разным для разных людей, проектов и стадий одного проекта. К счастью, этот процесс
- Однонаправленный, т.е. однажды выделенная ответственность никуда не пропадает.
- Конечный — имеет пределом однострочник.
И дальше в книге длина функции клеймится как самый страшный грех
Совершенно верно: длина функции — страшный грех, ибо является самым простым и заметным индикатором множества потенциальных и уже реализованных проблем в коде.
Если человек (а не кодогенератор) оставляет после себя длинную функцию, то ему де-факто безразличны любые проблемы с ее дальнейшим сопровождением.
Как это часто бывает, «что-то одно» означает один уровень абстракции какой-то логики (обычно бизнес-логики).
Нет, не означает. Введение уровней абстракции — это ограничения на зависимости между функциями.
И меня сильно утомляет осмысление многословных имён функций (и переменных), встраивание их в мысленную модель, решение, какие изучить подробнее, а какие пропустить, и наконец сложение всех кусочков мозаики в единую картину.
Здесь была злая шутка про связь национальности и стиля программирования автора статьи. Хорошо декомпозированная функция читается лучше, а если декомпозиция проведена плохо — это не повод отказываться от нее вообще, как предлагается в статье.
Напротив, когда в код добавляется много функций, почти неизменно теряется локальность.
На самом деле нет. Пока имя функции читается лучше, чем ее тело — с локальностью все очень хорошо.
Приверженцы маленьких функций почти неизменно стремятся передавать им меньше аргументов. Проблема в том, что из-за этого возрастает риск сделать зависимости неявными.
Это дезинформация в чистом виде. Неявные зависимости возникают совсем не от желания уменьшить число аргументов, а от готовности сделать это любой ценой.
Простой код необязательно легче в написании, и в нём вряд ли будет применён принцип DRY. Нужно потратить кучу времени на обдумывание, уделить внимание подробностям и постараться прийти к самому простому решению, одновременно верному и лёгкому в обсуждении. Самое поразительное в этой труднодостижимой простоте, что такой код одинаково легко понимают и старые, и новые программисты, что бы ни подразумевалось под «старыми» и «новыми».
Это точно: все одинаково легко понимают, что такой код — неподдерживаемый шлак.
Мне встречались в Ruby классы с 5-10 маленькими методами, каждый из которых делал что-то очень простое и брал в качестве аргументов один-два параметра. Многие из этих методов изменяют общее глобальное состояние или зависят от неявно передаваемых им синглтонов, что можно расценить как антипаттерн.
А вот за такое принято бить канделябрами. Изменять глобальное состояние плохо, только маленький размер методов здесь ни при чем.
Чем больше классов/интерфейсов/пакетов, тем труднее разобраться со всем этим одним махом, и ничто не оправдывает затраты на поддержание всех этих построенных нами классов/интерфейсов/пакетов.
Это очень вредная дезинформация. Разобраться в одном интерфейсе с 40 действительно очень просто — его надо выкидывать в мусорку сразу. Но поддерживать 40 интерфейсов с 1 методом в каждом намного проще.
Ларчик открывается просто — неважно, сколько у вас интерфейсов и классов, но важно, сколько зависимостей у каждого из них.
Пока это число невелико (0-3) — нет никаких проблем в навигации по сколь угодно большому графу программных объектов.
Итог: cтатья в целом — эталон крайне ядовитой демагогии, полезна для изучения именно в таком качестве.
Я избавился от размножения запросов к БД по всему коду, но сляпал ОГРОМНУЮ функцию, которая работает с базой. Она занимает едва ли не половину всего кода, но по сути выполняет лишь одну задачу — переформатирует строку запроса (по шаблонам SQL) согласно полученным данным при вызове и собсно общается с сервером БД. Всё. Читается элементарно, не смотря на размер. Должен ли я дробить её на сотню однострочников типа БазоПаролеМенятор, БазоЮзероУдалятор, БазоПиноЦоколятор, БазоБазоСоздаватор, БазоКурсороВозвращатор, БазоПиноПереименоватор…?
Не думаю.
Я хоть и эникейщик, но вряд ли сильно заблуждаюсь в убеждении, что единственное реально важное правило при организации функции: KISS. Абстракция реального мира — дыхание. Я не считаю нужным осознавать, какими именно мышцами между какими именно рёбрами надо совершить контракцию и ретракцию, когда и насколько надо напрягать диафрагму, какой объём воздуха всасывать и какие альвиолы наполнять. Это ОДНА, пусть и большая, но монолитная функция на том уровне абстракции, на котором она и должна быть — просто не забывай вызывать эту функцию с нужной частотой и ты будешь жить.
Ещё одно следствие избытка маленьких функций, особенно с очень описательными и неинтуитивными именами — труднее искать по кодовой базе. Функцию createUser грепать просто, а функцию renderPageWithSetupsAndTeardowns (это имя взято в качестве яркого примера из книги Clean Code), напротив, не так просто запомнить или найти. Многие редакторы поддерживают нечёткий поиск по кодовой базе, поэтому избыток функций с похожими префиксами наверняка приведёт к очень большому количеству результатов в выдаче, что трудно назвать идеалом.
Когда же вы уже, наконец, перестанете грепать и начнёте пользоваться нормальным семантическим поиском? Да, в ряде языков эта задача осложнена хитровы… вернотостью конфигурации подключаемого кода, но не к месту помянутая вами "многословная" java хороша как раз тем, что контекст там строго специфицируется и семантический поиск выполняется легко и быстро. Что, кстати, позволяет писать имена проще:
например не.вычислитьРазмерЭтогоЯблока а Яблоко.вычислитьРазмер. И да, то самое "загрязнение классами" так же позволяет писать более конкретные вещи.
Приверженцы маленьких функций почти неизменно стремятся передавать им меньше аргументов.
Проблема в том, что из-за этого возрастает риск сделать зависимости неявными.
И какие глобальные состояния соответствуют принципу "единственной ответственности" (single responsibility)? Т.е. из-за собственного нарушения контракта концепции вы объявляете концепцию плохой? Как-то странно мне это видеть.
Я уже говорил об этом выше, но стоит напомнить: многочисленные маленькие функции, особенно длиной в одну строку, чрезмерно затрудняют чтение кодовой базы.
Говорить можно много о чём. У кого-то вообще дислексия. Что с ними делать?
А функцию, содержащую проверку + вывод (которые можно тем же тернарным оператором запихнуть в одну строку) — я бы однострочной не называл. И да, бывает выгодно такие функции выделять, чтобы разгрузить чтение кода.
PS смешались в кучу люди-кони...
- ругаемся на DRY
- ругаемся на мелкие функции (и даже про это озаглавили)
- почему-то вдруг ругаемся на моки (а в том и идея модульного тестирования — что нам важно проверить текущий модуль, а не его взаимодействие с внешними!)
- ругаемся на ООП (да, это всё ещё модно)
- ругаемсяНаВыдуманныеПроблемыВысосанныеИзПальца (ну или актуальные для людей с дислексией… но в любом случае функция с именем, состоящим из более чем 3х слов — редкость, из более чем 5и слов — крайняя редкость)
При этом суть претензий: "фанатизм — это плохо". Ну супер, чо.
PS да, я вижу, что это перевод, но на медиуме я не зарегистрирован и толстому и зелёному автору оригинала ответить не могу.
Как и догматичные цитаты с критикой других догм (у которых, кстати, все-таки есть более развернутая аргументация в книгах упомятуных авторов).
Конечно, однострочные функции часто являются не лучшей практикой, но критиковать на их основе весь принцип DRY странно. Повторы в коде, содержащие некую логику, которую интуитивно не хочется инлайнить, обычно говорят о неких сущностях или отношениях, скорее всего существующих сейчас в неявных понятиях.
Отсюда получается основной лейтмотив статьи, который уловил лично я и с которым не могу согласиться — дублируемый код хорошо легче подвергается изменениям и добавлению функцианальности, чем моделирование бизнеса через абстрации.
Логично выделять в функцию ту часть кода, которая используется в нескольких местах.
Был случай как один сотрудник написал супер абстрактный фреймфорк для визардов, так им никто не смог пользоваться, т.к. для начала работы нужно было написать и рекцию на то или иное действие — полная абстракция
Spinning up and tearing down a database/queue
это не "Разгон и обрушение базы данных/очереди", а запуск и останов.
Все таки программирование немного искусство и советовать художнику
может стоит пользоваться научным методом а не в художников играть?
Научный метод не панацея, мозги включать все равно нужно.
можете пояснить мысль? То есть "научный подход" == "выключены мозги"?
в смысле исходный код красив
до первого чейндж реквеста. А так в целом ни разу не видал такого. Ну либо это было моментами. Сегодня смотришь — красиво. Завтра — чернь какая-то. В целом нам не за код платят.
За результат конечно. Однако звучит забавно, программисту и не за код. Просто приятно писать хорошо и неприятно плохо, да и нечестно в конце концов наваливать мусор в коде, даже если его никто не увидит
«До первого реквеста»
Дизайн недоработан, спек не понят или не продуман или ещё 1001 причина
Научный подход… Вы знаете сколько мне пришлось прочитать бредокода написанного людьми его исповедующего. Причём чаще всего он использовался для отмазки, почему нет результата или он неудовлетворителен, что впрочем не отменяет случаев когда он был использован к месту
Впрочем Вы и сами со всем этим сталкивались
Насчёт сабжа. Зависит от назначения и области применения программы. Иногда лучше мелкие функции по 2—3 строки иногда понятней фунции в несколько сотен строк, правда так бывает редко. Впрочем, часто это просто вопрос вкуса
Просто приятно
поймите, "красота" кода проявляется исключительно в способности с легкостью и минимальными рисками вносить изменения в систему.
Дизайн недоработан, спек не понят или не продуман или ещё 1001 причина
То есть если мы возьмем сферический проект где у нас есть четкое ТЗ, которое никогда не меняется, можно быстро и красиво запилить что-то и возможно даже это что-то будет красивым.
Но реальность такова что люди очень плохи в формулировании своих желаний. Более того, зачастую когда клиент хочет "кнопку и что бы она вот так вот делала", он на самом деле не кнопку хочет. А хочет проблему решить. И если мы проблемы не понимаем — 10 раз придется поменять что бы мы наконец пришли к чему-то нормальному.
Если на проекте более одного стэкхолдера, то поток изменений выходит уже весьма солидный. А существуют проекты настолько масштабные в плане заинтересованных лиц, что они могут даже не знать всей системы в целом (потому некоторые практикуют штуки типа event storming, что бы люди сначала между собой определились как что работает в их бизнесе). Ну и добавим сюда "новый бизнес", это когда мы вообще понятия не имеем что будем делать. Тут очень важно "быстро" подстраиваться.
Вся философия Agile движения (не та скрамная чушь а те люди что манифест составляли) построена на принципе "иногда больше думать не эффективно и нужно ставить эксперементы и собирать реальные данные".
Научный подход… Вы знаете сколько мне пришлось прочитать бредокода написанного людьми его исповедующего.
Вы под "научным подходом" подразумеваете "паттерны и принципы"? Научный подход подразумевает то, что вы ставите под сомнение общепринятые нормы и пытаетесь разобраться в фактах. Анализируете источники информации, делаете и главное проверяете предположения. А то что вы описали больше похоже на слепое следование каким-то правилам прочитанным в статях по типу "SOLID за 10 минут".
Я знавал ребят которые проектировали системы используя SOLID (да и я сам таким грешил какое-то время). Я только спустя время осознал что это в первую очередь инструмент для рефакторинга, а не проектирования. Причем работают они только при наличии потока изменений. Без этого потока сложно анализировать эффективность принимаемых решений. Это как предположения без проверки. Полная противоположность тех самых научных подходов.
Иногда лучше мелкие функции по 2—3 строки
опять же — вопрос не в количестве строк. Размер понятие относительное. Да и "хорошо" или "плохо" — не очень то инженерные понятия.
или хорошим инженерам, чем ремесленникам, которые делают одинаковые гайки по техзаданию.
вот инженер — это немного ближе к делу.
все попытки дать численные критерии качества работы программистов пока заканчиваются неудачей или провалом организаций их использующих
проблема не с метриками а их интерпритацией. Люди вводят простые метрики потому что на них проще ориентироваться, думать никто не хочет. Но только метрики и ограничения на них это не правила. Это скорее быстрый способ в общей массе находить потенциально интересные места на которых уже стоит человеку сосредоточить внимание а не тупо запрещать коммит.
Та же штука с покрытием кода тестами например. Ну пишет вам 100%, а мутационные тесты скажут что реальное покрытие 10%. Цель то этой метрики не в том что бы убедиться что все покрыто, а что бы быстро находить код вообще не покрытый тестами, и там уже решать, покрывать или нет.
Архитекторы, алгоритмисты, инженеры или кодеры. Все они пишут код больше или меньше. И требования к их коду и к ним самим все таки разные. И для какой области? Веб, игрушки или медицинское оборудование. Я работаю в последней больше 15 лет.Требования к надежности кода весьма высокие. Код просматривается неоднократно и разными людьми. Быстро писать не надо — надо надежно и понятно. Поэтому мне мой опыт сложно переносить в другие области, есть инерция мышления. И все равно я утверждаю, что хороший код должен вызывать эстетическое удовлетворение, как и картина или хорошая книга. В каком стиле пишет автор кода, мне без разницы, главное чтобы было понятно и прозрачно и если можно то и красиво
но пока в метологиях ИТ научным методом и не пахнет.
Что вы имеете ввиду под "методологиях ИТ"?
толпы кукарекающих евангелистов
Научный метод как раз и призван минимизировать эффект от "кукареканья", разве нет? Критическое мышление там, анализ фактов и т.д.
Одна только существование идиомы «модный фреймворк» чего стоит.
Мода есть везде и на все. Это к вопросу о том что люди чаще всего работают в рамках чего-то общепринятого и редко выходят за эти рамки.
Книги о том как надо писать код, например тот же «Совершенный код».
Вспомним начало 20-ого века. Были десятки книг на тему "как все устроено". А потом случилась ультрафиолетовая катастрофа и вдруг оказалось что те учения к которым все привыкли — они по факту не работают.
О дрейфе континентов человечество узнало только в 50-х годах 20-ого века. При этом идея сама была сформирована задолго до этого, просто люди посчитали это глупостью.
"Совершенный код" — это сборник лучших практик, которые собрал один человек. Это отличная книга, на самом деле, но на не код учит писать, а скорее дает вам совет. Следовать этим советам или нет — решать вам. На начальных этапах обучения когда вы еще плохо понимаете зачем оно надо — лучше все же следовать. Вам же далеко не сразу объяснили что на ноль делить на самом деле можно? А вот задавать вопросы "почему так а не иначе", это будет хорошо. И на них можно получить ответы, и найти альтернативные мнения.
Получается что это либо мода, либо все еще хуже — даже у программистов кто громче кричит того и слушают.
потому я и о говорю о развитии в людях критического мышления. Увы программисты тоже люди и проблемы у них те же.
Practical API Design
Although it took me a while, after a few years of designing APIs and working with others to design APIs that I liked, I came to realize that API design can be approached as an engineering discipline. Although it’s not obvious at first, this discipline does have a lot of objective attributes. As a consequence, it’s possible to turn the API design process into something that engineers can understand—a kind of science.
Every real discipline needs a theory behind it: a theory that defines the world that forms the subject of its domain. Such a world needs to be sharp and clearly defined. The less sharp it is, the less rigorous the science is, and the more it becomes an art.
Вообще грань между искусством и инженерией в книге неплохо раскрывается. Но для понимания этой грани совсем не обязательно читать талмуды — искусство в программировании — это красивый синоним для тупого брутфорса вариантов, когда мы не знаем как делать. Когда есть мат. аппарат — мы знаем как делать и искусство превращается в ремесло. Для первопроходцев поиск решения был искусством, для тех кто шел за ними — ремеслом.
Нигде не приведено, что же такое DRY.
По существу же статьи — кардинально не согласен с пунктом про имена.
Имена — это как раз самое большое достоинство маленьких функций, потому что вместо того, чтобы вникать, что именно делают хитросплетения конструкций языка — достаточно просто прочитать название!
Если же названия получаются чересчур длинными — то это признак того, что у вас проблема в архитектуре, или в используемом языке:
Не должно быть функций renderPageWithSetupsAndTeardowns — должны быть функции
renderPage(Setups, Teardowns)
Не должно быть
with setups and teardowns
в данном контексте лишние по семантике, это название экспоузит лишние детали. Но ничего страшного в методах типа predictRemnantOfIntermediateAccount
и т.д. нет. То есть опять же "размер" и т.д. тут определяются не количеством символов а семантикой.
А еще длинные функции могут появиться как следствие сложности придумывания названий. Например когда вы еще не знаете какой-то более короткой формулировки или названия термина но все еще пытаетесь описать что тут происходит. Если у разработчика проблема с языком — то еще сложнее.
It is better to have 100 functions operate on one data structure than 10 functions on 10 data structures. —Alan Perlis
Дело не в размере функций.
1000 процедур и 1 глобальная структура содержащая 100 полей?)
Дело не в размере функций.
и не в их количестве, и не в количестве структур данных. Просто это все намного проще "посчитать" а значит проще заставить начинающих следовать каким-то догмам. Гораздо сложнее например померять характеристики по настоящему влияющией на дизайн системы.
.
![image](https://habrastorage.org/webt/ne/u-/8d/neu-8dycato7frqlag5cxrolx_0.jpeg)
Мне кажется вообще немного не правильно говорить о функциях только в контексте их размеров. На мой субъективный взгляд, правильнее руководствоваться декомпозицией задач. Другой разговор, что не всегда возможно, уместно, удобно и нужно дробить большие функции.
«Наш мир все еще не идеален» или «нет серебряной пули», или «во всем нужна мера» сказал бы капитан.
SICP очень хорошая книжка, которая так же много чего говорит о значении абстракции.
Другой разговор, что не всегда возможно, уместно, удобно и нужно дробить большие функции.
Структурный дизайн по большому счету именно об этом и вещает. Проблема же в том что многие хотят сразу получить идеальное разделение (которого к слову нет), и в этом ключе можно тогда еще Рефакторинг Фаулера и "Эффективная работа с легаси кодом" добавить. Эти книги проповедуют "эволюционный дизайн". К сожалению для того, что бы эти подходы работали, в команде должны быть люди с довольно высоким уровнем личной ответственности.
Код читают не чтобы почитать, а чтобы разобраться и исправить. Понятное название позволяет решить, надо или нет разбираться в этой части кода. И вот решаешь что надо проверить этот метод в стороннем коде, заходишь туда, а там 2 строки с такими же понятными названиями уровнем пониже. Заходишь в каждый, а там тоже по 2 метода. Так просто сложнее искать ошибку.
Дело не в количестве строк, а в контексте. Одни говорят, что маленькие функции хорошо, другие что плохо, а имеют в виду разные ситуации. Метод
isActive()
из одной строки вполне нормально выглядит.А скажем такой код уже не очень. Гораздо понятнее, если все части алгоритма будут вместе.
static void someimpl_update(SOMEIMPL_CTX *c, const void *data, size_t len)
{
const unsigned char *ptr = data;
size_t res;
if ((res = c->num)) {
res = someimpl_update_first_part(res, c, ptr, len);
ptr += res;
len -= res;
}
res = someimpl_update_second_part(len);
len -= res;
if (len) {
someimpl_update_third_part(c, ptr, len);
ptr += len;
}
if (res) {
someimpl_update(c, ptr, res);
}
}
size_t someimpl_update_first_part(size_t res, SOMEIMPL_CTX *c, const unsigned char *ptr, size_t len)
{
res = someimpl_update_recalc_res(res);
if (len < res)
res = len;
someimpl_update(c, ptr, res);
return res;
}
size_t someimpl_update_second_part(size_t len)
{
return len % SOMEIMPL_CBLOCK;
}
size_t someimpl_update_recalc_res(size_t res)
{
return SOMEIMPL_CBLOCK - res;
}
void someimpl_update_third_part(SOMEIMPL_CTX *c, const unsigned char *ptr, size_t& len)
{
someimpl_block_data_order(c, ptr, len / SOMEIMPL_CBLOCK);
c->Nh += len >> 29;
c->Nl += len <<= 3;
if (c->Nl < (unsigned int)len)
c->Nh++;
}
Одни говорят, что маленькие функции хорошо, другие что плохо
но вы описали не проблему размера функций, а проблему зависимости между ними. Это немного другая история и с этим у разработчиков в среднем не так хорошо как хотелось бы.
потому функции надо объединять в модули. А получившиеся модули в другие модули. И делать так что бы модули выделяли какие-то функции более общие которые позволят нам иметь минимум точек взаимодействия между этими модулями. И да, это сложно.
И, значит, нарушает KISS.
Делать просто сложно :) делать просто — нарушает KISS
Делать просто — не может нарушать KISS, потому что это _и есть_ KISS :)
И, да, делать просто — просто, в этом весь смысл. Если вам делать что-то сложно — значит, вы делаете что-то непростое. Не надо этого делать :)
И, да, делать просто — просто, в этом весь смысл.
Самое простое что можно делать это взять код, и приписать сверху то, что вам надо. Правда, результат будет сложным. Сделать так, чтобы результирующий код был простым сложнее — надо разобраться в том, что вам надо и что уже сделано до вас и отрефакторить чтобы результат был простым.
KISS это когда легко пользоваться, точнее, когда реализация минимизирует риск неверного использования. И уж поверьте это не "просто" так сделать. Пример — USB Type A и USB Type C, каким проще пользоваться с точки зрения пользователя? Какой проще сделать было?
А «использование кода» (мы ведь о коде?) — это, в первую очередь, его чтение, все остальные варианты использования, в сущности, встречаются пренебрежимо редко. И если у вас какая-то сложная неинтуитивная структура зависимостей — то это плохо, потому что плохо читается. Даже цельная простыня читается лучше.
Кто знает, для популярных анализаторов, когда считают длину функций из суммы количества строк/оппераций вычитают инструкции/строки относящиеся к local/inner functions?
Есть смысл соблюдать DRY, только когда логика, которую можно обобщить, уже выражена в коде. Если логика для обобщения находится только в голове у автора, скорее всего, полученный в итоге DRY-подобный код будет сложно переиспользовать, он будет казаться чужеродным в остальных частях приложения. По-сути это одна из форм проявления преждевременной оптимизации.
чтобы не быть голословным, ниже код, писал не так давно, упростил для комментария:
![Register Example](https://habrastorage.org/webt/li/zq/2j/lizq2j94cedolpeszfadfffwox4.png)
![Extensions Example](https://habrastorage.org/webt/sk/ib/mk/skibmkdawgrjjpufamrhy0t31pk.png)
![Instruction Example](https://habrastorage.org/webt/wt/g5/cl/wtg5clbyzjgrrslyihusm6ewh0k.png)
благодаря следованию мудрым советам Дяди Боба, в частности SRP (также и в контексте методов) и правильному именованию
+ резко снижается шанс допустить ошибку, и легче найти и исправить в случае чего
+ код тестопригоден
+ код легко читается и понимается, всё очевидно
+ нет надобности в комментариях
и ещё один плюс, код в примере критичен по отношению к производительности, и не могу сказать про все языки и\или виртуальные машины (или отсутствие оных), но в моём случае, поскольку это .NET
+ CLR в рантайме просто отлично инлайнит короткие методы, в итоге мы получаем всё плюсы описанные выше без негативного влияния на производительнось
быстро, красиво и удобно, PROFIT!
код легко читается и понимается, всё очевидно
Вот про это и написано в статье, что далеко не всегда мелкие функции приводят к легко понимаемому коду. А по таким комментариям кажется, что всегда.
В вашем коде у компонента независимое API, поэтому там неважна длина методов. А вот если вы метод ADD() разобьете на 4 функции, то понять работу и найти ошибку в логике будет сложнее.
А по таким комментариям кажется, что всегда.
Цель то не в том что бы функции были маленькими, а в том что бы SOLID принципы соблюдались, а они раскрывают другие цели. Статистически просто если мы пишем маленькие функции — этот код с большей вероятностью будет соблюдать S (но бездумное уменьшение всего и вся или соблюдение этих принципов только в масштабах юнитов кода (классы, функции) а не модулей/компонентов/подсистем может привести к dependency hell.
По поводу ADD() вы просто не знакомы с кодовой базой моего проекта, по «выдранному из контекста» коду это не так очевидно и там на самом деле ещё есть что «выделить», правда разделив выполнение на 3 а не на 4 части ).
Методов, подобных инструкции ADD() много (как я уже писал больше сотни), и подавляющее большинство из них таких как INC(), DEC(), ASL(), LSR(), AND(), ORA(), и пр. выполняются по одной и той же схеме:
1. манипулялии с одним набором флагов
2. непосредственно операция
3. манипулялии с другим набором флагов
Инструкции не вызываются непосредственно, они вызываются из коллекции по коду операции (индексу) в зависимости от исполняемого эмулируемой системой кода.
Сейчас все эти приватные методы, представляющие инструкции просто последовательно объявлены в классе и «вбиты» в инициализвтор коллекции (Action[255]).
Т.е. получается такая хорошая «простыня» на почти 700 строк практически однотипного кода.
Я думаю из описания становится очевидно, что необходимо в будущем, когда вернусь к этой части системы создать класс, например «Instruction», и ннкапсулировать в него последовательное выполнение предварительных действий, с проверкой предусловий, т.е. на какие флаги влияет инструкция, потом выполнение непосредственно тела в большинстве случаев это будет однострочное лямбда выражение, и постусловий (так же как и с предварительными).
Для вызывающего кода наружу будет только один метод вроде «Execute()», и всё это положить в какой-нибудь класс типо «InstructionSet», который выполнит первоначальную инициализацию (конструирование всех инструкций, можно например из xml-ки десериализовать или прямо в коде проинициализировать), а так же по обращению из «ядра» по «опкоду» выполнять инструкцию из внутренней коллекции. Инициализацию и выполнение можно так же отделить, но я думаю в моём случае пока в этом нет необходимости, потом будет видно.
В результате избавимя от «простыни» дублирующегося кода.
По факту, мелкие функции действительно бывают огромной проблемой, но только потому, что выделяют не те фрагменты и не в те места.
Действительно все крутится вокруг SRP. И хоть в ООП данный подход и приводит к появлению множества различных уровней абстракций, при правильной реализации «осознать» такой код значительно проще, чем кашу из методов (будь они хоть длинными, хоть короткими).
Каждая функция — абстракция. Собственно, мысль автора — если мы принудительно ограничиваем размер функций, то это порождает массу плохих (непродуманных) абстракций.
Но намного проще сказать людям делать маленькие функции нежели попросить их правильно абстракции выделять. Некоторые (например Сэнди Мэтс) умышленно идут на подобное упрощение. Правда она потом поясняет и зачем, и связь с абстракцией, и предупреждает о возможных проблемах, но людям проще делать маленькие функции по количеству строк.
Сказать-то проще, но зачем? Если вы так скажете, то в итоге как раз и получите то, о чем пишет автор статьи. Использование маленьких функций — это инструмент, а не цель. И человек должен понимать, зачем этот инструмент применяет, а иначе это будет просто карго-культ с закономерным результатом.
И хоть в ООП данный подход и приводит к появлению множества различных уровней абстракций
Ну как сказать, мне кажется это распространенное заблуждение и следствие неправильного проектирования. Хотя быть может я вас плохо понял.
Маленькие функции VS большие функции? Как по мне они должны быть средними по размеру. Разве это не решает все проблемы?
На мой взгляд решение такое: кода должно быть меньше. Если разбивание на мелкие ф-ии увеличивает объём кода — это очень плохо. Если разбивание на более мелкие ф-ии уменьшает объём кода — это очень хорошо. При этом под объёмом кода понимается и количество строк и ментальная нагрузка, которую этот код создаёт.
Проще — лучше. Но только до тех пор, пока код решает те задачи, которые перед ним ставятся.
Абстракции — это хорошо. Ровно до тех пор, пока они уменьшают объём кода. И не стоит забывать, что объём кода — это и ментальная нагрузка. Если вы разбиваете код на много мелких частей, то не стоит забывать о сложности связей между этими частями.
Кроме того надо не забывать о производительности и оптимизациях. Но тут стоит напомнить о бенчмарках (без них можно даже не начинать). И о том, как этот код будет использоваться. Если ты будешь писать несколько часов такой код, который отработает за секунду, вместо того, чтоб потратить пять минут на код, который будет работать десять… Эй! Это же утилита, которую ты запустишь один раз! Другими словами оптимизировать код нужно по тем совокупным ресурсам, которые на него затрачиваются сегодня и завтра. Иногда будет дешевле написать супероптимизированный код. Но далеко не всегда. Кстати во многих случаях простой тупой код с парой хаков будет работать быстрее, чем сложная запутанная система. Но разумеется есть исключения.
И в целом код должен делать то, что от него хотят. И не больше. Иногда твоей задачей будет написание кода, который противоречит твоим религиозным взглядам на то, как идеальный код должен выглядеть.
Универсальный совет: прежде чем лететь к архитектурным звёздам или погружаться в глубины оптимизаций, следует провести пятиминутный анализ того, а зачем оно вообще нужно.
Абстракции — это хорошо. Ровно до тех пор, пока они уменьшают объём кода. И не стоит забывать, что объём кода — это и ментальная нагрузка. Если вы разбиваете код на много мелких частей, то не стоит забывать о сложности связей между этими частями.
Вопрос, какой объем кода считается — общий? Или на конкретном уровне абстракции?
Разбиение документа на части заголовками увеличивает объем, но разобраться в результате проще, потому, что на верхнем уровне есть ограниченный объем глав, и можно понять где заканчивается конкретная глава.
Они должны быть понятными.
Размер — это средство, а не цель. Но многие это путают.
Парадоксально абстрактная статья о вереде абстракции.
Nix Solution видимо наняла копирайтера по методологии Porozjnyak.
В защиту «Чистого кода» скажу, что автор книги с самого начала пишет о недогматичности своих рассуждений и просит в первую очередь руководствоваться здравым смыслом. Вся книга это не сборник правил, а скорее тру стори о внесении ясности в существующий код. Она должна стать началом вашей собственной истории вдохновлённой чужим опытом, коррелирующим со своим собственным. А никак не очередным шаблоном для нового культа Карго с постепенным переходом к обратному [культу Карго].
В противовес ей эта статья polnayaMetodovVisasonyhIzPalcaKotorieSlozhnoChitatIzZaIhDlinyhImen (автор чистого кода не рекомендует так делать) и граблей из слепого следования тезисам. Всё это похоже на обратный культ Карго. Синди как бы намекает, что те кто пишет лаконичные функции сами танцуют на граблях, но так умело, что никто этого не понимает кроме неё, немогущей прочитать, что за ними кроется.
Дорогой переводчик сего эпоса. Прочитай сам «Чистый Код», а не полагайся на выжимки глупого треугольника. Это задача не из лёгких, автор пишет об этом в самом начале (для меня было оптимальным чтение перед сном, отлично усыпляет).
Но, если ты ещё не натанцевался на граблях и не чувствуешь, что тебе сложно читать свой собственный код, написанный несколько месяцев назад — танцуй! Иначе не хватит мотивации, чтобы её дочитать. Вся книга — история оптимизации настоящего не абстрактного кода, целью которой является его ясность, а не следование конкретным характеристикам.
Как пример — «разбить функцию из 60 строк на 4 функции по 15 строк» из этой статьи. Это ведь реально сложный абстрактный пример не применимый в реальных условиях. Делить нужно на необходимое количество функций в зависимости от их логической основы, в не от результирующего количества строк в каждой из них.
А Yuniy Padavan, что? Верен словам мастера и делит функцию на ноль четыре идеально равных части. Без сомнений, чего бы это ни стоило. Ну и потом разочарование с переходом на байткод дарксайд.
P.S. Я понимаю негодование Синди Шридхаран, сам встречал людей упоротых тезисами и превозносящими их над здравым смыслом. Возможно если бы она не писала настолько абстрактно, то получилась бы годная статья о вреде слепого следование правилу один — «Функция должна быть компактной» и правилу два — «Функция должны быть ещё компактней». Но вышло не только без достаточной конкретики, так ещё и с превозношением копипасты и деградации.
Кстати книга Роберта Мартина начинается так:
Кстати в другом вашем переводе встречается такой тезис:
Хорошей привычкой является написание небольших функций — не более 10 строк.
Сравните с утверждением — «Функция должна быть компактной».
Это тот случай где абстракция побеждает конкретику. Функция функции функция.
Например, от хорошей привычки доведённой до автоматизма, попытка поделить функцию которой нужно занимать больше десяти строк (например только дестркутор в начале функции может быть в восемь строк. Его, что? В отдельную функцию выписывать?) приведёт к чрезмерному усложнению кода, который станет в последствии сложно читать и породит статью Синди Шридхаран.
Что такое "деструктор в начале функции"?
render () {
const {
props: {
theme_name,
updateMethod
},
state: {
account_number,
contract_number,
contract_date,
inn,
kpp,
card,
sum
},
onSubmit,
onUpdateObject
} = this
return(…)
}
Не самый удачный пример, но в качестве примера деструктора достаточно (эту функцию можно разбить на подкомпоненты, но не со всеми так получится)
Понятно. В обычном императивном коде строки более насыщены (т.е. тут фактически каждый параметр на отдельной строке, при просто вызове функции будет несколько переметров на одной. Больше имеют значение семантические единицы, а не синтаксические
Этой штукой мы как бы декларируем с чем будем работать метод и извлекаем из объекта необходимые свойства. Это избавляет от необходимости постоянно писать вложенные пути:
this.props.someMethod(this.state.deep_array.value)
someMethod(value)
Первая строка без использования деструкторизации. Вторая куда приятнее.
При разработке небольших приложений в пару сотен строк можно не морочиться с ООП, разделением на файлы и работой в команде.
С другой стороны если команда работает целый год над своей частью кода, от императивного подхода все довольно скоро начнут страдать (особенно если пишут все в один файл), а когда попросят всё переделать ответят — «Только с нуля переписывать»
От себе скажу, что даже для не очень длинных задач теперь предпочитаю ООП императивному стилю, а раньше конечно простыни катал, но устал.
По поводу консистентности, можно было написать так:
render () {
const {
props: {theme_name, updateMethod },
state: { account_number, contract_number, contract_date, inn, kpp, card, sum},
onSubmit, onUpdateObject
} = this
return(…)
}
но уменьшать количество строк в ущерб читаемости как-то не айс. Это повлияет на производительность, причём не компьютерную, а мою.
Вред маленьких функций