FiresShadow 9 окт 2014 в 08:11

Миф о идеальном количестве строк в методе

8 мин

20K

Программирование * Проектирование и рефакторинг *

Из песочницы

115

Комментарии 115

avsej 9 окт 2014 в 08:42

… об идеальном…

avsej 9 окт 2014 в 13:31

Всего двое неграмотных? Неплохо

Amper 10 окт 2014 в 05:50

Думаю, что дело не в грамотности, а в том, что такие вещи нужно писать личным сообщением, а никак не в первой ветке комментариев.

turbo_exe 10 окт 2014 в 07:03

какие вещи? автор всё правильно употребил.

ОБ, предлог.
1.
Употр. вместо «о» (2.О):
перед словами, начинающимися с гласной, например: об армии, об искусстве, об отце, об угол;

http://www.gramota.ru/slovari/dic/?word=%EE%E1&all=x

НЛО прилетело и опубликовало эту надпись здесь

FiresShadow 9 окт 2014 в 11:21

«А можно немного конкретики?»
Я как раз и постарался привести пример на псевдокоде, когда все действия находятся на одном уровне абстракции и не удастся разбить их на методы.

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

Sild 10 окт 2014 в 05:30

Айпи как-раз предоставляют практически все сайты.

FiresShadow 10 окт 2014 в 05:49

Тут опечатка. По смыслу понятно, что имеется ввиду апи (прочитайте предыдущее предложение)

Colwin 30 окт 2014 в 08:40

Лучше писать англоязычные сокращения на английском, и вас поймут правильно и однозначно: API.

basilisk 10 окт 2014 в 15:45

Практика показала, что огромные методы всегда имеют серьезные проблемы

Есть подозрение, что в таких случаях часто путают причину и следствие.

Как правило, все огромные методы, что мне попадались, это не причина серьёзных проблем, а способ решения серьёзных проблем. Реализация запутанного алгоритма, как правило, увы, не получается простой и лакончной. Отсюда и длинные портянки запутанного кода. Попытки разделять это на части проваливаются. В лучшем случае получается сделать как в топике у автора: « 1) выполнить первую часть ритуального танца; 2) выполнить вторую часть – и так далее»

ApeCoder 11 окт 2014 в 12:13

Можете привести пример. Только реальный?

basilisk 11 окт 2014 в 17:40

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

Последнее, с чем приходилось сталкиваться на практике, так это слияние нескольких DOCX/ODT документов. С сохранением логической структуры документа, разумеется.

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

Реализация этого слияния поначалу была простой и логичной. А дальше понеслось… Самое забавное, что на данный момент уже несколько не самых слабых разработчиков пытались всё это оптимизировать, упростить, переписать, разбить на части. Через пару дней до людей начинает доходить, что тут не всё так просто и клёвый алгоритм, оказывается не работает, или не учитывает ещё какой-либо позабытый момент. Или же получается, увы, то же самое что и до попыток оптимизации.

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

ApeCoder 11 окт 2014 в 19:27

Мне кажется не надо сливать XMLНоды, должен быть слой, который транслирует понятия документов на XML.

Colwin 30 окт 2014 в 08:42

… и придем к XSLT — тот еще хаос. :-)

ApeCoder 30 окт 2014 в 09:07

XSLT не знает о предметной области — это он работает в терминах XML нод. Я имел ввиду чтобы можно было сказать.

Cell.Value  = "Hello"

не парясь со списком shared strings и прочим

ApeCoder 9 окт 2014 в 08:47

1) Узнать информацию о поставщике;
2) Узнать информацию о лотах закупки;
3) Узнать информацию о аукционе закупки;
4) Узнать информацию о протоколах закупки;
5) Узнать информацию о последней дате изменения состояния закупки.

Я думаю, если подумать, такое можно вполне разбить на фазы: у нас есть данные, которые имеют структуру например

Аукцион
   - лоты 
   - закупка
       - дата изменения состояния
       - протоколы

Соответственно эту иерархию можно перенести в код.

Интересно было бы разобрать реальный пример.

wheercool 9 окт 2014 в 09:35

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

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

ApeCoder 9 окт 2014 в 09:55

Хотелось бы разобрать конкретный пример все-таки. Можно привести такой пример?

Для меня красная тряпка когда добавляют комментарии в длинный метод разбивая его на части типа:

// разбор заказов
.. 5 шагов..
// разбор лотов
... 8  шагов ...

Вместо того, чтобы просто разбить на более короткие методы

FiresShadow 9 окт 2014 в 10:24

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

ApeCoder 9 окт 2014 в 10:50

Я так и понял. Можете на ОО языке (хотя бы на UML) описать как связаны понятия

Поставщик;
Закупка;
Аукцион;
Протокол;
Дата последнего изменения.

dimview 10 окт 2014 в 00:57

Тут ключевой момент не в длине метода, а в повторении кода.

Если код разбора заказов и код разбора лотов очень разный, то нет смысла делать два метода.

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

ApeCoder 10 окт 2014 в 02:12

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

По ссылке написано про модули, а не по методы.

Colwin 30 окт 2014 в 08:49

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

Привести пример второго типа сложно, но мне два-три раза такое попадалось.

И еще один пример: не нужно устранять дублирование, если оно является частью разных бизнес-сценариев, подверженных изменениям. Завтра сценарии могут разойтись — и общие части придется опять распиливать. А вовремя сдублировать ветку кода для независимой модификации — этого не умеют делать многие миддлы, не говоря уже о Junior'ах. Как раз потому, что знают принцип «устранения дублирования», но не понимают его суть.

MacIn 10 окт 2014 в 02:27

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

ApeCoder 10 окт 2014 в 04:05

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

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

MacIn 11 окт 2014 в 14:04

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

— мы не увидим в стектрейсе этого названия

Если просто в callstack'е при отладке — не страшно, мы и так видим, где мы сейчас. Если при exception'е — есть инструменты, которые выдают номер строки (у нас используется Eurekalog для Delphi, в лог при обвале идет callstack с номерами строк в модулях).
В общем, зависит от используемых инструментов.

— мы не сможем свернуть его в редакторе

Ни разу не приходилось сворачивать. Видимо, по-разному с кодом работаем.

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

Вполне можно использовать переменные с ограниченной областью видимости (в С++, например) или определять переменные для каждого блока свои и не использовать их повторно. На стеке столько же места займет.
Насчет закомментированного — предпочитаю следовать правилу — никаких закомментированных блоков в репозитории. Комментарии — для комментариев, для остального — система контроля версий.

— нет возможности декларировать исходные данные и результаты для каждого куска

Очень общее утверждение, сложно что-то обсуждать.

Для меня прокрутка менее удобна чем охватывание одним взглядом одним взглядом :)

В общем, это сугубо индивидуально. Непонятно, за что понаставили минусов — ведь просто мнениями обменялись. Выделение кода в отдельные модули также «раздувает» сам исходник. Зачастую получается, что процедурка, которая практически полностью влазила в экран, разбивается на куски и из-за заголовков/блоков часть кода уползает еще дальше. И если поначалу не видно было пару строк, но вся логика работы куска кода была как на ладони, то теперь надо прыгать по процедурам туда-сюда, последняя(ие) из которых уже в экран не влазят совсем.
Но посыл автора считаю верным — не стоит выделять код в процедуру просто потому что слишком много строк.

ApeCoder 11 окт 2014 в 15:59

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

Изначальный смысл безразличен, главное — как удобнее.

Если просто в callstack'е при отладке — не страшно, мы и так видим, где мы сейчас.

Не видим, комментарий может быть сверху за границами экрана.

Если при exception'е — есть инструменты, которые выдают номер строки (у нас используется Eurekalog для Delphi, в лог при обвале идет callstack с номерами строк в модулях).
В общем, зависит от используемых инструментов.

Осталось по номеру определить комментарий, которым был помечен блок и отличить его от комментария к конкретной строке.

— мы не сможем свернуть его в редакторе

Ни разу не приходилось сворачивать. Видимо, по-разному с кодом работаем.

Тогда если у вас есть N блоком прокоментированных сверху

// Разбор файла
// Преобразования
// Запись

Как увидеть общий план?

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

Вполне можно использовать переменные с ограниченной областью видимости (в С++, например) или определять переменные для каждого блока свои и не использовать их повторно.

1. Я не очень хорошо знаю C++ он позволяет писать так?

// разбор файла 
{
   handle = readFile(name)
   .... куча кодв
   close (handle)  
}

2. Ключевые слова — язык не контролирует то есть если вы и будете использовать уникальные переменные для блока легко ошибиться.

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

Я имел ввиду комментированный блок — блок, предваряемый комментарием.

— нет возможности декларировать исходные данные и результаты для каждого куска

Очень общее утверждение, сложно что-то обсуждать.

Customer parseFile(string fileName)
{
... куча кода
}

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

// parsing file
... куча кода

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

MacIn 11 окт 2014 в 16:25

Изначальный смысл безразличен, главное — как удобнее.

Каждый инструмент для своей задачи.

Не видим, комментарий может быть сверху за границами экрана.

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

Осталось по номеру определить комментарий, которым был помечен блок и отличить его от комментария к конкретной строке.

Зачем? Если разбивка на блоки идет с таким же размером, как для методов, комментарий — в зоне видимости. Да его и вовсе может не быть, если код понятен сам по себе, дались вам эти комментарии :)

Как увидеть общий план?

Просматриваем метод сверху вниз, просто скроллингом. Вее видно, все потоки управления, и скакать по процедурам туда-сюда не надо.

Я не очень хорошо знаю C++ он позволяет писать так?

Пример, даже не скомпилируется:
{
int i;
i = 5;
cout << i;
}

cout << i;

2. Ключевые слова — язык не контролирует то есть если вы и будете использовать уникальные переменные для блока легко ошибиться.

Приведите пример.

трудно понять что код берет в качестве исходных данных, что возвращает,

Ну почему же? В вашем примере, если блок предварен чем-то вроде «парсим файл, находим клиента», и при этом есть переменные FileName и Customer, то понять несложно. Если еще использовать что-то вроде венгерской нотации (вроде), то еще проще: например, у нас в команде принято все поля данных в методе предварять F, все локальные переменные — l, формальные параметры — A. тут будет уже lCustomer и AFileName.

вообще говоря непонятно, к чему относится комментарий, к первой строке, к 10 строкам или к чему

Ну, как — к следующему блоку кода, отделенному либо комментарием, либо пустой строкой. Комментария может и вовсе не быть.

Я не против разбивки на методы, это было бы глупо. Я против разбивки ради разбивки.

ApeCoder 11 окт 2014 в 16:36

Не видим, комментарий может быть сверху за границами экрана.

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

Далее — заголовок метода у меня в колстеке (такое окно в дебагерре) а комментарий вообще может быть не в этом окне.
(например я поставил точку останова в функции MessageBox и вижу колстек main \ ParseFile \ MessageBox а не main \ MessageBox — чтобы увидеть, что брейкпоинт сработал именно при разборе файла, мне надо открутиться вверх до main по колстеку, а потом открутиться вверх до // parse file в редакторе )

2. Ключевые слова — язык не контролирует то есть если вы и будете использовать уникальные переменные для блока легко ошибиться.

Приведите пример.

// parse file
....
int i = MaxParseIterations;
while (i>0)
{
}
...
// transform
...
while (i < 1000 )
{
}
...

Ну почему же? В вашем примере, если блок предварен чем-то вроде «парсим файл, находим клиента», и при этом есть переменные FileName и Customer, то понять несложно. Если еще использовать что-то вроде венгерской нотации (вроде), то еще проще: например, у нас в команде принято все поля данных в методе предварять F, все локальные переменные — l, формальные параметры — A. тут будет уже lCustomer и AFileName.

То, что вы описали для меня — за гранью.

В-общем, компилятор и тулзы они хорошие — если им рассказать побольше — они помогут, а коментариев они не понимают.

MacIn 11 окт 2014 в 17:53

Далее — заголовок метода у меня в колстеке (такое окно в дебагерре) а комментарий вообще может быть не в этом окне

Его может и вовсе не быть. Но код-то вы вызывающий видите.

например я поставил точку останова в функции MessageBox и вижу колстек main \ ParseFile \ MessageBox а не main \ MessageBox

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


// parse file
....
int i = MaxParseIterations;
while (i>0)
{
}
...
// transform
...
while (i < 1000 )
{
}
...

Используйте IterationIdx и TransformIdx, в чем проблема?

То, что вы описали для меня — за гранью.

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

В-общем, компилятор и тулзы они хорошие — если им рассказать побольше — они помогут, а коментариев они не понимают.

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

Опять же — это основано на личных предпочтениях, как уже выяснилось выше.

ApeCoder 11 окт 2014 в 19:30

Мы рассматриваем вот это:

Для меня красная тряпка когда добавляют комментарии в длинный метод разбивая его на части типа:

То есть метод разбит на логические куски коментариями

Используйте IterationIdx и TransformIdx, в чем проблема?

В том, что компилятором не контроллируется.

MacIn 12 окт 2014 в 00:07

И не надо. Код вы пишете, а не компилятор. С таким же успехом вы можете утверждать, а вдруг, мол, мы используем по ошибке имя не локальной переменной, а поля данных класса. Что ж делать-то, компилятор же это не контроллирует!

То есть метод разбит на логические куски коментариями

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

ApeCoder 12 окт 2014 в 05:57

И не надо. Код вы пишете, а не компилятор. С таким же успехом вы можете утверждать, а вдруг, мол, мы используем по ошибке имя не локальной переменной, а поля данных класса.

Типа «если компилятор не проверяет x пусть он не проверяет и y» — хорошая логика ;)

MacIn 21 окт 2014 в 00:13

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

FiresShadow 9 окт 2014 в 10:06

Вы правы, я действительно имел ввиду, что длина метода напрямую зависит от количества составных шагов в доменной модели для данной user story. Также я согласен с вами, что накручивание синтетических абстракций поверх доменной модели приведёт лишь к отходу от единого языка (Ubiquitous Language), которым пользуются как специалисты предметной области, так и программисты, что в итоге сведёт на ноль использование DDD в проекте и усложнит сам проект. Однако доменная модель в зачаточном состоянии используется и теми, кто даже ни разу не слышал о DDD. Проектируя программу в стиле ООП, программист является отчасти и специалистом предметной области, пытаясь разбить программу на модули, соответствующие некоторым объектам реального мира или знакомым программисту абстракциям (например, матриц) из предметной области. Поэтому всё сказанное справедливо не только для DDD, но и для любого дизайна в ООП стиле. Придумывать синтетическую абстракцию поверх доменной модели, которая создаётся естественно и интуитивно при таком проектировании, зачастую значит идти против себя и против тех, кто будет заниматься сопровождением кода.

ApeCoder 9 окт 2014 в 10:43

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

FiresShadow 9 окт 2014 в 13:56

Если тот, кто выдаёт вам user-story (спецификацию программы), не умеет умножать, действительно следует задуматься о том, чтобы завести отдельную функцию «Сложить m между собой n раз», а внутри этой функции использовать умножение. Умный JIT-компилятор оптимизирует ваш код, в результате чего быстродействие не пострадает. В C++ можно объявить метод как inline. При наличии такого метода, вам не придётся напрягать лишнюю извилину мозга, когда заказчик произносит: «и тут нужно сложить степень двойки этого полиномиального коэффициента между собой записанное в таком-то файле количество раз». Вы просто вызовете нужную функцию для вашего уже вычисленного коэффициента и только что прочитанного значения из файла. Однако умножение — очень примитивная абстракция, выражающая через другую очень примитивную абстракцию (сложение). Если же вам придётся переводить сложную абстракцию постановщика user-story в непонятно для чего изобретённую вашу сложную абстракцию, то ваш мозг быстро закипит. В результате вы напишите гораздо меньше кода с гораздо большим количеством ошибок. Советую вам почитать книгу о DDD Эрика Эванса.

ApeCoder 9 окт 2014 в 15:50

«Сложить m между собой n раз» — это умножение. На его языке это звучит так:
«Прибавить к двум два. К результату еще прибавить два, потом еще раз прибавить два.»

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

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

FiresShadow 9 окт 2014 в 17:25

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

ApeCoder 9 окт 2014 в 19:11

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

При формализации этого никак не избежать. Надо называть вещи которые ранее были смутным пятном в мозгу эксперта.

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

Поэтому и прошу примера. То есть я согласен, на 100% сказать, что ВСЕ методы длинее, допустим, полэкрана плохие нельзя, но для длинных методов нужно хорошее осознанное обоснование и они являются исключением.

FiresShadow 11 окт 2014 в 10:31

Добавил пример в конец статьи

ApeCoder 11 окт 2014 в 12:22

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

Например, от это 16 раз почему — 16, а не 32 или 7.

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

FiresShadow 11 окт 2014 в 12:56

«А то получается, что автор метода не вы»
Вы правы, я не изобретал DES.

«Например, от это 16 раз почему — 16, а не 32 или 7.»
Боюсь, мне понадобится около месяца не ходить на работу и всё это время изучать глубины математики и криптографии, чтобы понять почему 16 итераций цикла посчитали достаточным количеством для достижения необходимого уровня криптостойкости при существующем уровне вычислительных мощностей компьютера, которые могут быть использованы для перебора ключей. После этого вам потребуется примерно такое же время, чтобы понять мои объяснения. Вы согласны договориться с моим работодателем, оплатить материальные издержки и пообещать, что мои усилия не окажутся напрасными и вы будете в этом разбираться? Попросту говоря, я не знаю почему 16 =)

FiresShadow 11 окт 2014 в 13:02

Хотя, наверное, 16 как-то связано с размерностью блока в байтах и количестве информации, которая в этот блок влазит, но не уверен, так что обманывать не стану

ApeCoder 11 окт 2014 в 13:10

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

ApeCoder 11 окт 2014 в 13:09

То есть здесь явно есть две проблемы:
1) Настоящие авторы метода написали непонятно
2) Вы не понимаете как он сделан и почему (именно потому, что 1) )

То есть в данном случае «длинный метод = проблема» срабатывает.

FiresShadow 11 окт 2014 в 13:55

«Настоящие авторы метода написали непонятно»

То что для одного непонятно, для другого — очевидно. Программист в своей карьере может сталкиваться с различными предметными областями, и на освоение всех из них в совершенстве ему просто не хватит жизни. Вот вы понимаете, почему ядро атома не разлетается на кусочки в то время как внутри него действуют чудовищные отталкивающие силы? А если вас попросят запрограммировать модель атома, будет ли для вас проблемой то что вы это не понимаете, точно также, как для вас проблема, что вы не понимаете почему в DES-е 16 итераций? Нет, вы скорее всего просто возьмёте и запрограммируете.

«То есть в данном случае «длинный метод = проблема» срабатывает»

Было бы неплохо, если бы вы хоть как-то обосновывали свою точку зрения. Наверное, вы считаете, что так как а)«Чем меньше метод, тем он понятнее» и б)«Этот метод непонятный», ТО «если понаделать мелких методов, станет понятнее». Или, может быть, вы считаете, что «раз алгоритм не дробится на мелкие части, то это плохой (непонятный) алгоритм». На самом деле, непонятный не значит плохой, непонятный тут значит чуть сложнее обычного. Ну вот встречаются в природе такие алгоритмы, у которых на верхнем уровне абстракции много действий, и ничего с этим не поделать, кроме как самому напридумывать синтетических абстракций. Самый простой способ эти абстракции придумать — разбить алгоритм на «Функция имени меня», «Функция имени вон той планеты», «Функция имени моего домашнего животного», а потом завести книжку с описанием, что в какой функции происходит, потому как «Функция Фейстеля» из DES-а ну никак не отражает содержания этой функции. И если раздробите вы плохо декомпозирующийся алгоритм на части, понятнее он от этого не станет. Примеры я уже приводил.

ApeCoder 11 окт 2014 в 14:47

То что для одного непонятно, для другого — очевидно

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

Вот вы понимаете, почему ядро атома не разлетается на кусочки в то время как внутри него действуют чудовищные отталкивающие силы? А если вас попросят запрограммировать модель атома, будет ли для вас проблемой то что вы это не понимаете, точно также, как для вас проблема, что вы не понимаете почему в DES-е 16 итераций?

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

Самый простой способ эти абстракции придумать — разбить алгоритм на «Функция имени меня», «Функция имени вон той планеты», «Функция имени моего домашнего животного»,

4.3 Основная функция шифрования (функция Фейстеля)

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

Фактически, я бы написал алгоритм так (псевдо F#)

let DES x = x |> splitToBlocks |> encryptBlock |> joinBlocks
let encryptBlock x = x |> initialPermutation |> encryptLoops mainEncryption |> finalPermutation

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

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

Авторы статьи в википедии это уже за вас сделали — зачем вы уничтожили эту структуру в своем псевдокоде?
Почему вы называете плохо декомпозирующимся алгоритм, хотя сами признаете что его не понимаете?

Примеры я уже приводил.

Приведено три примера:
1) Разбор сайта — не является длинным методом (всего пять строчек) — длинный метод не приведен. Приведите пожалуйста более длинный код.
2) Хаотический аборигенский танец — не понятен контекст преобразования танца в метод, скорее всего является фантазией автора а не реальной задачей (подтвердите, пожалуйста, или дайте больше контекста).
3) DES — в википедии уже разбит на стадии (наверное авторам так было понятней? ;)) почему-то решили что лучше знаете как описывать DES и слепили это в один комок.

Я пропустил что-то в этом списке?

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

FiresShadow 11 окт 2014 в 15:09

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

Я вот только что, в 18-15 дал ответы на все эти вопросы, а вы в 18-47 снова их задаёте, только вставляете их так, чтобы они были выше в ветке комментариев. Скажите честно, вы — тролль?

Повторюсь в очередной раз, 3й пример — это немного модифицированный DES, а не сам DES.

ApeCoder 11 окт 2014 в 16:07

Я не тролль, я отвечал на пачку комментариев по порядку и не обновил, извините :)

Повторюсь в очередной раз, 3й пример — это немного модифицированный DES, а не сам DES.

Извините, не понял. где модификация DES — вы заменили функцию для шифрования полублоков на свою?

ApeCoder 11 окт 2014 в 13:26

Еще одна проблема вы зачем-то переносите из википедии код не 1-в-1 а инлайня функции. В википедии написано как раз лаконично

Схема шифрования алгоритма DES
4.1 Начальная перестановка
4.2 Циклы шифрования
4.3 Основная функция шифрования (функция Фейстеля)
4.4 Генерирование ключей
4.5 Конечная перестановка

Вы, не понимая назначения шагов, зачем-то решили поменять их названия или вообще опустить (заметим, что «функция имени кого-то» написана в скобках после «Основная функция шифрования»)

Если же вам придётся переводить сложную абстракцию постановщика user-story в непонятно для чего изобретённую вашу сложную абстракцию, то ваш мозг быстро закипит. В результате вы напишите гораздо больше кода с гораздо большим количеством ошибок. Советую вам почитать книгу о DDD Эрика Эванса.

FiresShadow 11 окт 2014 в 14:15

«вы зачем-то переносите из википедии код не 1-в-1 а инлайня функции»

Это было сделано намеренно. Первая цель — показать, как авторы самого алгоритма занимались декомпозицией. Вторая цель — так как мы модифицируем алгоритм, содержимое «функции Фейстеля» изменяется, и это больше не «функция Фейстеля» (т.к. она делает другие операции). Соответственно, мы не можем использовать ту же самую декомпозицию и туже самую абстракцию, что использовали авторы DES, и толк от неё теряется.

«Циклы шифрования»
Что выражает название метода «Циклы шифрования»? Что это что-то, как то связанное с шифрованием и что внутри есть цикл. То что это связано с шифрованием, итак ясно из контекста использования метода. То что в названии функции отражено что внутри есть цикл — это вообще нарушение инкапсуляции. Это как раз пример плохой синтетической абстракции, о которой я говорил ранее. Это всё равно что дать методу имя «Сделать хорошо», а классу — «Manager» или «Service». Эти названия ничего не отражают.

ApeCoder 11 окт 2014 в 15:42

«вы зачем-то переносите из википедии код не 1-в-1 а инлайня функции»

Это было сделано намеренно.

Ок мы не понимаем алгоритм но при этом отказываемся использовать обозначений от тех, кто понимает.

Первая цель — показать, как авторы самого алгоритма занимались декомпозицией. Вторая цель — так как мы модифицируем алгоритм, содержимое «функции Фейстеля» изменяется, и это больше не «функция Фейстеля» (т.к. она делает другие операции).

Мы можем ее переименовать на «зашифроватьПолублок»

Соответственно, мы не можем использовать ту же самую декомпозицию и туже самую абстракцию, что использовали авторы DES, и толк от неё теряется.

Толк простой — для сопоставления с исходным опиванием не надо искать какие-то паттерны в коде, а надо просто сопоставить названия частей.

«Циклы шифрования»
Что выражает название метода «Циклы шифрования»? Что это что-то, как то связанное с шифрованием и что внутри есть цикл. То что это связано с шифрованием, итак ясно из контекста использования метода. То что в названии функции отражено что внутри есть цикл — это вообще нарушение инкапсуляции. Это как раз пример плохой синтетической абстракции, о которой я говорил ранее.

1. Название «Лопата» никак не отражает его предназначения, но проще использовать его, чем каждый раз говорить «Штык и древко для копания» — со временем слово лопата запомнится и статен привычной.Мы можем вводить понятия и давать им неговорящие названия, это тоже имеет ценность больше, чем не давание названий. например со словом «кварк» проще чем его не иметь и каждый раз расшифровывать это понятие.
2. Давайте декомпозируем эти циклы я бы представил это так:

let encryptionStep encryptBlockHalf (Left,Right) key = Right, (Left xor (encryptBlockHalf key)) 
let encryptionLoops block encryptBlockHalf keys = keys |> fold (encryptionStep encryptBlockHalf) (splitBlock block)

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

можно ее назвать, например, encryptBlockByHalves.

Это всё равно что дать методу имя «Сделать хорошо», а классу — «Manager» или «Service». Эти названия ничего не отражают.

Тут вопрос, что лучше, если не знаешь как придумать название — не давать никакого и не делать никакой структуры или дать какую-то структуру. Мне кажется лучше дать какую-то структуру — даже если имя с первого раза ничего не будет говорить, при последовательном обращении обозначения запомнятся и проще будет находить соответствие. Например, я сейчас сопоставляю английскую и русскую версию статьи про DES и вижу, что:
1. С русской работать лучше так как структура вынесена в оглавление.
2. Название функции Фейстеля повзоляет легко сопоставить части алгоритма.

FiresShadow 11 окт 2014 в 17:13

«со временем слово лопата запомнится и станет привычной»

Похоже, всё дело в том, что не понимаете что такое Абстракция. Абстракция — это отвлечение от несущественных сторон, с целью выделения существенных, закономерных признаков. Ну вот разбили вы алгоритм на «Кусок1» и «Кусок2», каждый из кусков используется только один раз. Какой у «куска1» закономерный признак? Он используется в модифицированном DES-e. Ну и что, второй тоже там используется. Нет закономерных признаков — нет абстракции. Кроме того, ваша «абстракция» и место где она используется в вашем случае — одно и тоже. Тому, кто будет сопровождать ваш код, эта «абстракция» никак не поможет — чтобы понять что это такое, нужно посмотреть что внутри метода. А чтобы понять что там внутри, нужно проваливаться внутрь метода. Это вредит дизайну программу, а почему вредит — я подробно рассматривал в статье.

FiresShadow 11 окт 2014 в 19:22

Перечитал свой ответ на ваш комментарий, и понял что он недостаточно подробно отвечает на главный ваш вопрос.

«Тут вопрос, что лучше, если не знаешь как придумать название — не давать никакого и не делать никакой структуры или дать какую-то структуру.»

Перечитайте то место в статье, где говорится о последствиях, когда имена методов не отражают назначения методов.

Ну да ладно, в очередной раз повторюсь. Как говорится, повторение — мать учения.
Если вы каждый день работаете с кодом, то поневоле вы может и выучите, что есть в «Куске1», а что в «Куске2». Но раз вы наизусть знаете алгоритм, то если содержимое «Куска1» и «Куска2» вставить в одно место, то вы всё равно найдёте нужное вам место. А вот если пришёл новый разработчик на проект, то он видит кучу методов с ничего не выражающими для него именами, при этом код разбит на мелкие процедуры и ветвист. Ему нужно обходить дерево кода как лабиринт. Или вот хочет новый разработчик внести изменения в «Кусок1», но сомневается как это повлияет на «Кусок2», заходит он в «Кусок2», а там «Подкусок1» и «Подкусок2»… Работать в таких условиях очень тяжело. Разбор кода идёт медленно, при кодировании часто допускаются ошибки. Автору то кода и названий может и хорошо живётся, более того, он — «Незаменимый программист», потому как никто «почему-то» не может нормально работать с тем что он понаписал, а поддерживать и сопровождать всё это надо, поэтому «Незаменимый программист» — ценный сотрудник. Более того, создаётся ложное впечатление, что «Незаменимый программист» более опытный, потому как он может, а другие почему-то — нет.

ApeCoder 11 окт 2014 в 20:23

Я совершенно согласен с тем что название должно отражать назначение и с тем, что «циклы шифрования» не очень хорошее название.

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

Во-вторых, давайте разберем исходный пример по подробнее:

# 1)Разбить исходный текст на блоки по 64 бита
def modifiedDES(stream, key):
     #2)Для каждого такого блока
     blocks = split_to_blocks(stream)
     for block in blocks:
          encryptBlock(block, key) 
# {
def encryptBlock(block):
    #  2.1)Переставить биты в блоке местами по определённому алгоритму
    initialPermutation(block)
    #  2.2)Разбить блок на два блока длиной 32 бита (далее — левый и правый)
    (left, right) = splitBlockToHalves(block)
    # 2.3)Повторить 16 раз:
    subKeys = generateSubkeysFrom(key)
    for subKey in subKeys:
         #  {
         #    2.3.1)Вычислить Ki из ключа шифрования K и номера итерации по определённому алгоритму.     
         #    2.3.2)Высичлить блок E длиной 48 из правого блока длиной 32 по определенному алгоритму.
         expansion = expand(right)
         #    2.3.3)F = побитовая сумма Ki и E
         F = subKey xor E
         #   2.3.4)левый блок длиной 32 = правый блок длиной 32 бита на прошлой итерации
         #  2.3.5)правый блок длиной 32 = левый блок 32 на прошл итерации, побитово сложенный с F
         (left, right) = (right, left xor F)
      #  2.3.6)Добавить левый блок в конец результата шифрования
      result.write(left)
      #   2.3.7)Добавить правый блок в конец результата шифрования
      result.write(right)

Итого

# итерация 1
def modifiedDES(stream, key):
     blocks = split_to_blocks(stream)
     for block in blocks:
          encryptBlock(block, key) 
def encryptBlock(block):
    initialPermutation(block)
    (left, right) = splitBlockToHalves(block)
    subKeys = generateSubkeysFrom(key)
    for subKey in subKeys:
         expansion = expand(right)
         F = subKey xor E
         (left, right) = (right, left xor F)
    result.write(left)
    result.write(right)

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

Следующий шаг «4.3 Основная функция шифрования (функция Фейстеля)»

Анализ — функция получает подключ и половину блока, выдает преобразованную половину блока. Мне кажется вполне
логично назвать ее шифрованием половины блока (encryptBlockHalf)

# итерация 2
def modifiedDES(stream, key):
     blocks = split_to_blocks(stream)
     for block in blocks:
          encryptBlock(block, key) 
def encryptBlock(block):
    initialPermutation(block)
    (left, right) = splitBlockToHalves(block)
    subKeys = generateSubkeysFrom(key)
    for subKey in subKeys:
         (left, right) = (right, left xor encryptBlockHalf(subkey, right))
    result.write(left)
    result.write(right)  
def encryptBlockHalf(subKey, blockHalf):
    # основная функция шифрования
    return subKey xor expand(blockHalf)

Самая длинная функция — 7 строк

Итерация 3 — Циклы шифрования
Циклы шифрования берут набор подключей и функцию шфврования полублока и последовательно ее применяют к полублокам пары по отношению к текущему ключу:

Вход: подключи и функция шифрования половинки;

# итерация 3
def modifiedDES(stream, key):
     blocks = splitToBlocks(stream)
     for block in blocks:
          encryptBlock(block, key) 

def encryptBlock(block):
    initialPermutation(block)
    subKeys = generateSubkeysFrom(key)
    encryptBlockByHalves(block, subkeys, encryptBlockHalf)

def encryptBlockByHalves(block, subkeys, encryptBlockHalf):
    (left, right) = splitBlockToHalves(block)
    for subKey in subKeys:
         (left, right) = (right, left xor encryptBlockHalf(subkey, right))
    result.write(left)
    result.write(right)  

def encryptBlockHalf(subKey, blockHalf):
    # основная функция шифрования
    return subKey xor expand(blockHalf)

Самая длинная функция — 5 строк.

ApeCoder 11 окт 2014 в 20:39

Если вы каждый день работаете с кодом, то поневоле вы может и выучите, что есть в «Куске1», а что в «Куске2». Но раз вы наизусть знаете алгоритм, то если содержимое «Куска1» и «Куска2» вставить в одно место, то вы всё равно найдёте нужное вам место.

Не факт — вместро ctrl+F Кусок1 придется смотреть глазами. С другой стороны, можно постепенно изучая давать более удачные обозначения.

«Кусок1», но сомневается как это повлияет на «Кусок2», заходит он в «Кусок2», а там «Подкусок1» и «Подкусок2»…

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

FiresShadow 12 окт 2014 в 06:40

«если есть устойчивое сложившееся название это может окупить его неправильность»
Именно так я и сказал в своей статье. Перечитайте пример с модифицированным DES-ом. В очередной раз повторюсь, что там мы немного модифицируем «Функцию Фейстеля», после чего она «Функцией Фейстеля» не является, так же мы модифицируем и некоторые другие места, поэтому устойчивых сложившихся названий в модифицированном DES-е нет.

«постепенно изучая давать более удачные обозначения»

Перечитайте то место в статье, где говорится, что метод плохо поддаётся декомпозиции, если «не получится объединить некоторые из них (строчки кода) в отдельную функцию с понятным именем». Повторюсь в очередной раз и дам более подробные объяснения. Есть метод «Узнать информацию о закупке», на наивысшем уровне абстракции у него одна операция — получить информацию о закупке, на уровне абстракции пониже (далее — верхний уровень) — столько операций, сколько внутри закупки других сущностей, информацию о которых нужно узнать. Т.е. там внутри 1)узнать информацию о лотах закупки 2)о протоколах 3)о отзывах 4)о прогнозах 5)о поставщике, и ещё много чего. И каждая строчка кода имеет цель, никак не связанную с другими строчками кода на верхнем уровне абстракции. Если бы была связь, то их можно и нужно было бы объединить в отдельный метод. А если нет связи, то не нужно объединять. А если вы всё-таки объедините в один метод и дадите методу имя «Кусок1», то сколько не думайте, удачного названия не найдёте — решаемые то задачи кода не изменятся от того, что вы провели декомпозицию.

«то достаточно легко узнать повлияет изменение на другие куски или нет»

Допустим, «Кусок1» изменяет состояние класса так, что это нарушает контракт «Куска2». Или результат вычислений «Куска1» передаётся в «Кусок2», нарушая его контракт. Как понять по названию «Куска2», какой у него контракт? Да никак. Второй пример — «Кусок1» был модифицирован, после чего оказалось, что «Подкусок1» из «Куска2» стал очень похож на «Кусок1». Не один в один, но если подумать немного, то можно сделать параметризацию и вынести дублирующийся код в отдельный метод. Как, даже имея перед глазами подробную UML-диаграмму, вы поймёте, что «Кусок1» и «Подкусок2» похожи? Никак. Если вы сводите на нет читаемость и самодокументированность кода только потому, что «что-то метод длинноват», то не ждите, что это останется без последствий.

«за счет выноса шифрования блока в отдельную функцию и лаконичных обозначений в стиле Pythion»

Вам удалось создать одну удачную абстракцию — Блок. Но вот функция «зашифровать блок» как раз таки будет большой. Я и не подразумевал, что весь модифицированный алгоритм DES-а нужно делать в одну функцию, наверняка получится выделить ещё абстракцию «Перестановка», принимающая в качестве параметра блок и матрицу перестановок. Но вот внутри модифицированного DES-а будут большие методы (зашифровать блок). Абстракция «Зашифровать полублок» — неудачная, т.к. полублок вы дешифровать не сможете (без второго полублока). Текст шифруется блоками, а не полублоками. По сути, метод должен называться «Произвести с полублоком действия, необходимые для шифрования блока». Название метода ничего не отражает, т.к. это итак ясно из контекста использования. А если бы в блоке каким-то чудом была матрица, вектор и лопасть вертолёта, вы бы предложили выделить методы «Сделать что-то с МатематичПонятиями» и «Сделать что-то с ЛопастьюВертолёта»? Мне не нравится такой дизайн, а почему — я уже говорил неоднократно.

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

ApeCoder 12 окт 2014 в 10:29

Абстракция «Зашифровать полублок» — неудачная, т.к. полублок вы дешифровать не сможете (без второго полублока)

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

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

.

е. там внутри 1)узнать информацию о лотах закупки 2)о протоколах 3)о отзывах 4)о прогнозах 5)о поставщике, и ещё много чего

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

В очередной раз повторюсь, что там мы немного модифицируем «Функцию Фейстеля», после чего она «Функцией Фейстеля» не является,

1. В Википедии используется термин «основная функция дотирования» причем по фестеля говорится в скобках

2. Даже если вы модифицируете функцию фейстеля, вы можете назвать ее «модифицированная функция фейстеля» и любому знакомому с des станет понятно, какое место она занимает а алгоритме

ApeCoder 12 окт 2014 в 10:34

Допустим, «Кусок1» изменяет состояние класса так, что это нарушает контракт «Куска2». Или результат вычислений «Куска1» передаётся в «Кусок2», нарушая его контракт. Как понять по названию «Куска2», какой у него контракт?

1. По какому угодно имени нельзя понять контракт можно только угадать его часть
2. Поэтому существуют способы для описания контактов кроме названия
3. Если кусок 1 не будет оформлен как функция, мы будем знать о контракте меньше

FiresShadow 13 окт 2014 в 11:16

«По какому угодно имени нельзя понять контракт можно только угадать его часть»

Лично у меня редко возникают сложности с тем, чтобы уместить контракт в фразу максимум из 6 слов (но обычно слов меньше, длинные названия в основном в тестах и описывают user story, проверяемые тестами), и по названию функции понятно что функция делает, и какие у этого последствия тоже ясно, поэтому и конструкции вида Contract.Assert() я редко использую.

«Если кусок 1 не будет оформлен как функция, мы будем знать о контракте меньше»

Не вижу сложностей с тем, чтобы заинлайнить метод с ничего не выражающим именем и не потерять сведений о контракте этого куска кода. Поскольку название ничего не выражало, то эта часть контракта не теряется (т.к. её и не было). Конструкции вида Contract.Assert() инлайнятся вместе с кодом.

ApeCoder 13 окт 2014 в 19:16

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

Конструкции вида Contract.Assert() инлайнятся вместе с кодом.

А параметры и возвращаемые значения?

Кстати, еще раз вопрос, почему «шифрование полублока» неправильный термин?

FiresShadow 14 окт 2014 в 14:54

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

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

На самом деле подобрать хорошее название методу, чтобы оно описывало контракт, легче, чем кажется. Сначала просто берёте и пишите что делает функция (это важно — что, а не как) со всем контрактом, включая описание требований к параметрам. Может получиться что-то вроде РассчитатьДебетИКредитДляВипКлиентаПроверивПраваНаДоступКДаннымКлиента(клиент). Сразу видно, что функция нарушает принцип единственности ответственности, разделяем её на РассчитатьДебетДляВипКлиента(клиент) и РассчитатьКредитДляВипКлиента(клиент), а проверку прав, как сквозную функциональность, выносим в отдельный аспект. Потом вдруг замечаем, что вторая функция может рассчитать кредит для любого клиента, изменяем её имя на РассчитатьКредит(клиент). РассчитатьДебетДляВипКлиента(клиент) оставляем в таком виде до тех пор, пока не потребуется рассчитывать дебеты для других типов клиентов, тогда можно будет добавить параметр isVip (или использовать полиморфизм), убрав пред-условие виповости клиента из названия. Также убираем из названия функции лишние постусловия, наступление которых очевидно. Если функция имеет название ДобавитьЭлементВКонтейнерУвеличивЕгоРазмерностьНаОдин(элемент), то так как очевидно, что добавление элемента в контейнер повлечёт увеличение размера контейнера на один, то из названия это постусловие убираем, остаётся ДобавитьЭлементВКонтейнер(элемент). Потом вспоминаем, что функция существует не в вакууме, а в классе Контейнер, поэтому урезаем название до ДобавитьЭлемент(элемент). Что именно добавляется понятно из параметров, поэтому остаёте просто Добавить(элемент). Если нет времени заниматься рефакторингом, то лучше использовать длиннющее или неинформативное название, чем обманчивое. Название метода «УзнатьДату» — наихудший выбор из возможных, если метод не только узнаёт дату, но и сохраняет её в бд, а также рассылает пользователям уведомления на почту при её изменении, хотя «УзнатьДату» внешне и выглядит приличнее, чем «ОбработатьДату» или «УзнатьИСохранитьДатуИУведомитьПользователейПриЕёИзменении».
Советы, как говорится, «от капитана очевидности», но может быть кому-нибудь окажутся полезными.

ApeCoder 14 окт 2014 в 15:55

Юнит-тесты же хороши для описания user-story

Для юзер стори разве на акцептанс/компонент/интегрейшен тесты

и для контроля соблюдения контракта, но я никогда не видел хороших примеров описания контракта тестами.

testCreateFile_shouldThrowException_whenFolderDoesNotExist

FiresShadow 14 окт 2014 в 16:28

«testCreateFile_shouldThrowException_whenFolderDoesNotExist»

Разве это нельзя назвать user-story?
На самом деле можно было бы и не бросать исключение, а создать директорию, тогда бы размер контракта уменьшился (т.к. избавились от одного предусловия), и функция выполняла бы свои обязанности, если она в состоянии их выполнить, а кидала бы исключение только в случае невозможного имени файла/директории. Если существование директории обязательно, можно отразить это в названии метода, например CreateFileInExistedDirectory, тогда вы дважды подумаете, прежде чем использовать этот метод в случае, если существование директории не обязательно.

ApeCoder 14 окт 2014 в 16:49

Что должно делать CreateFileInExistingDirectory, в случае, если такой папки нет?

ApeCoder 14 окт 2014 в 04:39

Шифрование полублока, это правда не шифрование, а функция раунда. Я бы назвал при описании DESа ее функцией Фейстеля, в а модифицированном DES — назвал бы просто round функцией:

// Блочный шифр - это шифр который шифрует локи отдельно
let encryptByBlocks encryptBlock source  = 
    source |> splitToBlocks |> map encryptBlock |> joinBlocks

// des - это блочный шифт
let des key = encryptByBlocks (encryptBlockUsingDes key)

// шифрование блока это первичная перестановка, потом шифрование 
// фейстеля с использованием функции фейстеля в качестве round функции
// потом конченая перестановка
let encryptBlockUsingDes block key = 
   block |> initialPermutation |> encryptByFeistel feistelFunction key |> finalPermutation

// Сеть фейстеля
let encryptByFeistel roundFunction key block  =
   let subKeys = splitKey key
   subkeys |> fold 
      (fun (left, right) subkey -> (right, left xor (rountFunction right))) 
      (splitBlock block)

// модифицированный DES это тоже блочный шифр
let modifiedDes key = encryptByBlocks (encryptBlockByModifiedDes key)

// Вместо функции фейстеля в качестве раунд фунции используем xor
let encryptBlockUsingModifiedDes block key = 
   block |> initialPermutation |> encryptByFeistel (^^^) key |> finalPermutation

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

Если сама сеть фейстеля будет модифицирована, то можно эту концепцию назвать «модифицированной сетью фейстеля» (например «упрощенная сеть фейстеля») или использовать понятие «product cipher»

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

Итого:

1. «Модифицированный DES» — это тот же «безсистемный танец» — то есть оторванный от реальности пример, так как мы не специалисты по шифрованию и не можем его осознанно модифицировать, мы не можем судить о его непригодности к декомпозиции.

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

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

FiresShadow 14 окт 2014 в 15:37

«Модифицированный DES» — это тот же «безсистемный танец»… так как мы не специалисты по шифрованию»

А это приводит к ещё одной интересной мысли — не стоит заниматься декомпозицией и выносить действия в отдельную метод не только когда эти действия не имеют общей цели (или имеют, но нам об этом неизвестно), но и тогда, когда действия имеют общую цель, но для осознания этого программисту потребуется больше усилий, нежели сэкономит декомпозиция. В самом деле, может быть шаман в курсе, что приседание и кудахтание вызовет восторг духа горы, а другие четыре действия привлекут внимание духа озера. Но если в планах нет написания серьёзного приложения, умеющего применять множество заклинаний, исходя из знания нужных комбинаций довольных богов, а на самом деле нужно просто сделать робота, владеющего парой танцев, то может ну их, эти аборигенские верования? Точно так же глубокое изучение криптографии, вплоть до понимания что общего у round функций всех шифров (а если нет ничего общего, то это никакая не абстракция), только ради того, чтобы декомпозиция модифицированного DESa приносила пользу, а не вред, — такое понимание тоже вряд ли окупится, если нужно написать всего один шифрующий алгоритм. И даже если вы знаток аборигенских верований и криптографии, для создания хорошего дизайна нужно в первую очередь думать о других программистах.

«Названия не всегда отражают назначение, иногда эта штука служит для запоминания/обозначения»

Я уже описывал вам своё мнение на этот вопрос в комментарии за 11 октября 2014 в 23:22

ApeCoder 14 окт 2014 в 16:13

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

Зачем портить существующую декомпозицию, зачем заниматья тем, в чем не разбираешься?

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

Оно принесет пользу сразу, как только начнешь ее так называть — просто потому, что по этому термину можно полгуглить.

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

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

Кстати, конструировать новый криптографический алгоритм без глубокого знания криптографии — это, ИМХО, запредельный ламеризм, от которого предупреждают во всех букварях.

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

Итого:
— Пример с разбором сайта не является маленьким, большой не приведен
— примеры про танец и про «модифицированный DES» нереальны

FiresShadow 14 окт 2014 в 16:42

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

ApeCoder 15 окт 2014 в 05:15

Для примера с сайтом нужно немного воображения,

— Не бывает здоровых кроликов красного цвета!
— Бывают, вот вам пример.
— Но это же белый кролик!
— А вы представьте, что он красный!

примеры с танцем и модифицированным DES-ом могут стать реальными при некотором стечении обстоятельств.

Почему вам удобнее обозначать ваш алгоритм как «модифицированный DES» вместо того, чтобы инлайнить его описание в каждом месте где вам надо употребить этот термин?

«Модифицированный DES» очень не точный термин — давайте вы сначала приведите все названия в соответствие со своими же воззрениями.

FiresShadow 15 окт 2014 в 15:10

Действительно, ПрименитьАлгоритмМодифицированногоDESа(данные) — действительно неудачное название для функции. Однако проблема не в том, что это не точный термин, в котором мало конкретики, а как раз наоборот — проблема в том, что в нём СЛИШКОМ много конкретики. Со временем, после множества модификаций, с DES-ом у неё может остаться мало общего. Также в нем могут быть использованы куски других алгоритмов шифрования. Более правильное название в этом случае — ПрименитьСобственныйАлгоритмШифрования(данные). А если в программе только один алгоритм шифрования, и других добавлять не планируется, то можно просто — Зашифровать(данные). Ну и если язык поддерживает ООП, то нужно занести шифрующую и дешифрующую функции в один класс. При этом в названии «Зашифровать» достаточно конкретики, контракт функции предельно ясен — данные преобразуются таким образом, что их практически невозможно восстановить без ключа. При этом как это происходит, пользователю функции не важно.

aisek 10 окт 2014 в 04:16

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

FiresShadow 9 окт 2014 в 10:18

На самом деле, вы поняли немного неправильно. Аукцион, лоты, протоколы, дата изменения закупки — всё это относится к закупке. Лоты не относятся к аукциону. Все эти сущности лежат на одном уровне абстракции. А из-за того, что они на одном уровне, и действия по их обработке также на одном уровне абстракции, то их и не удаётся скомпоновать в разные функции. Этот пример был специально так подобран. Это и есть реальный пример, только на псевдокоде

ApeCoder 9 окт 2014 в 10:41

То типа того:

class Закупка
{
      Дата датаИзменения;
      Аукцион аукцион;
      List<Протокол> протоколы;      
}

class SiteParseResult{
     Запкупка закупка;
     Лоты лоты;
}

SiteParseResult parseSite()
{
    return new SiteParseResult(){
           закупка = parseЗакупка();
           лоты = parseЛоты();
    )
}

Давайте пример на псевдокоде

FiresShadow 9 окт 2014 в 11:06

Лоты это часть закупки. Внесите лоты в закупку. После этого внутри parseЗакупка будет тот псевдокод, который описан в статье. Я не считаю хорошей мыслью разбивать закупку на два класса. Прочитайте пожалуйста обоснование в ответе на комментарий risik-а, который предлагает проделать тоже, что и вы, но только не с лотами, а с поставщиком. Не хочу засорять статью однотипными комментариями :)

FiresShadow 9 окт 2014 в 10:20

KeFA 9 окт 2014 в 09:53

Крайности — это, как правило, всегда плохо, а истина где-то посередине.

FiresShadow 9 окт 2014 в 10:13

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

KeFA 9 окт 2014 в 10:23

пытаться всегда быть где-то посередине — это тоже своего рода крайность.

Именно) Поэтому я и добавил «как правило»

PS Со статьёй я и не спорю, просто подытожил

Alexufo 9 окт 2014 в 21:15

определяете середину вы из определения крайностей. Тут никак без них) А они у каждого свои)

Colwin 30 окт 2014 в 09:12

Лучше стремиться к точке устойчивости.
Предварительно убедившись, что данное устойчивое состояние Вам подойдет. :-)

risik 9 окт 2014 в 10:27

1) Узнать информацию о поставщике;
2) Узнать информацию о лотах закупки;
3) Узнать информацию о аукционе закупки;
4) Узнать информацию о протоколах закупки;
5) Узнать информацию о последней дате изменения состояния закупки.

а) Здесь 5 строк кода. Это как раз меньше 10, но больше 3.
б) Это похоже на типичный пример последовательной связности (sequential cohesion). Которая может быть разбита на n процедур с тем же типом связности без особых потерь логики. Навскидку в данном конкретном случае я вижу разбиение на две процедуры: 1) Узнать информацию о поставщике 2) Узнать информацию о закупке.
в) ИМХО как раз для этого типа связности правило количества строк может не распространяться.

Количество строк в методе не является самоцелью. Это всего лишь индикатор.

FiresShadow 9 окт 2014 в 10:44

а)Здесь 5 строк кода. Это как раз меньше 10, но больше 3.

После перечисления этих 5ти методов написано… и ещё подобные методы, просто я не стал перегружать статью, отметив, что процесс торговли можно всё усложнять и усложнять, в результате чего количество сущностей на одном уровне абстракции разрастётся, разрастётся и метод, содержащий сейчас 5 строк кода. Можно наращивать метод неограниченно.

б)Навскидку в данном конкретном случае я вижу разбиение на две процедуры: 1) Узнать информацию о поставщике 2) Узнать информацию о закупке.

Нам нужно распарсивать информацию о поставщике, относящемуся к закупке. Внутри закупки есть ссылка на поставщика. Таким образом, поставщик — часть информации о закупке. Если разбить код на эти две функции, то когда через месяц, немного подзабыв код, вы снова его откроете, то будете неприятно удивлены, не обнаружив парсинг поставщика в функции «Узнать информацию о закупке». Согласитесь, что продавец — часть информации об процессе обмене ценностями между двумя людьми (организациями). Имя же функции «Узнать информацию обо всём в закупке, кроме поставщика», которое здесь уместно, лично мне кажется не лучшим выбором.

«Количество строк в методе не является самоцелью. Это всего лишь индикатор.»

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

ApeCoder 9 окт 2014 в 10:45

) Это похоже на типичный пример последовательной связности (sequential cohesion).

Имхо тут не последовательная связанность — так как часть шагов независимы (могут быть разбиты на параллельные таски)

ApeCoder 9 окт 2014 в 11:20

После перечисления этих 5ти методов написано… и ещё подобные методы, просто я не стал перегружать статью, отметив, что процесс торговли можно всё усложнять и усложнять, в результате чего количество сущностей на одном уровне абстракции разрастётся, разрастётся и метод, содержащий сейчас 5 строк кода. Можно наращивать метод неограниченно.

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

Я думаю, человеку не свойственно оперировать длинными списками и он все равно его как-то классифицирует (помните правило 8+-2).

Не могли бы вы привести более длинный пример в комментарии?

FiresShadow 9 окт 2014 в 14:18

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

ApeCoder 9 окт 2014 в 15:54

«аборигенский танец из ста хаотичных безсистемных действий» это не пример метода (нету кода). Хотелось бы понять зачем этот танец кодируют в виде метода, что ожидается в результате какие его подробности существенны, а какие нет (нельзя ли его заменить на sum(random(действие) for x in range(100))).

Разбор сайта пример понятный и я могу представить примерно что-там нужно а что нет, а аборигенский танец — не очень. Вы действиетельно занимались программированием аборигенов или пример нереалистичный?

harvester 10 окт 2014 в 04:39

Длинный алгоритм танца не повод его хардкодить. Вынесите его во внешний файл, читаете и вызываете нужные методы. Теперь метод превратился из простыни в 100+ вызовов в 2 блока, получить алгоритм, выполнить. Тоже самое с парсерами, это один и тот же пример, описанный разными словами. Имхо Вы перечитали ооп проектирования и Вам все кажется гвоздями, и да, опыт показывает, чем длиннее метод, тем выше вероятность, что писал на скорость.

FiresShadow 10 окт 2014 в 05:04

«опыт показывает, чем длиннее метод, тем выше вероятность, что писал на скорость»
Вообще вся статья и написана ради того, чтобы показать что длинные методы, как правило плохо, но — НЕ ВСЕГДА (неужели по первым трём предложениям статьи этого непонятно?!)

«Вынесите его во внешний файл»
Варианты с внешним DSL и нейросетью уже обсуждались РАНЕЕ(да и статья СОВСЕМ НЕ ОБ ЭТОМ). Пожалуйста, вставляйте свои предложения и пожелания в КОНЕЦ обсуждения, а не в середину, тем более что ваш коммент имеет мало что общего с тем местом, куда вы его вставили. Мне бы хотелось тратить время на ответы действительно заинтересованным людям, а не тем, кто не прочитал ни статьи, ни комментарии к ней…

ApeCoder 9 окт 2014 в 10:44

del

boolivar 9 окт 2014 в 13:03

Я бы предложил вам каждый шаг алгоритма выделить в данные, тогда реализация самого алгоритма будет занимать 3-5 строк цикла обработки данных, размер же самих данных в +100500 элементов, никого не смущает.

Что-то типа:

moves = {'присесть': aMove, 'покудахтать': bMove, 'порычать': mooMove, 'помахать руками': cMove, 'привстать': dMove, 'попрыгать': eMove,…};

magicDance(moves) {
for (move: moves) {
move.do();
}
}

И такая реализация, гораздо лучше метода в +100500 строк, теперь входные данные могут быть произвольными, генериться на лету, например во время танца под музыку.

FiresShadow 9 окт 2014 в 13:23

Вы предлагаете использовать внешний DSL. Об этом написано много статей на хабре. Т.е. где-то в коде описано как нужно приседать, и когда где-нибудь в конфигурационном файле встречается команда «присесть», вызывается код, который «знает» как приседать. Основная проблема в том, что не всё уместно реализовывать через DSL. На практике принято сочетать внешний DSL с обычным кодом, причём обычного кода как-правило на практике получается больше. Почему так — тема отдельной статьи. Повторюсь, что единственная цель этой статьи — развеять миф о том, что если в коде метода более чем n или менее чем m строк кода, то с методом ТОЧНО что-то не в порядке в плане проектирования. Разумеется, этот код можно сделать и лучше, и по-другому. Можно вообще завести нейронную сеть, которая будет в интернете скачивать картинки приседаний и учиться приседать автоматически. Но статья совсем не об этом. Можно сказать, что танцы никому не нужны; да и парсить сайт тоже не нужно, а нужно заставить разработчиков сайта написать API. Статья тоже не об этом. Указанные алгоритмы приведены как примеры. Хочется услышать ваше мнение (желательно обоснованное) по поводу того, миф это или нет. Однако за комментарий спасибо. Было бы ещё лучше, если бы вы указали название предлагаемой методологии (внешний DSL), чтобы те, кто не в курсе, могли более подробно с ней ознакомиться

boolivar 9 окт 2014 в 14:21

Да не имел в виду я никакой DSL, просто не хотел привязываться к конкретному языку программирования.
В си я бы использовал массив указателей на функции, в C++/Java накидал бы коллекцию различных реализаций интерфейса Move и т.д.
Да, можно вынести во внешние конфиги или описать действия на внешнем DSL, чтобы не пересобирать программу при изменении состава действий, в любом случае, это будет лучше чем метод с большим количеством строк.

FiresShadow 9 окт 2014 в 14:46

Мне непонятно, чем массив из указателей (без применения DSL) лучше непосредственного вызова самих методов. Мне кажется, что при таком подходе будет сложно вносить изменения в список параметров, сложнее отлаживать, да и разбираться в коде при таком подходе тоже капельку сложнее. А какие преимущества у данного подхода? Чем это лучше? Тем, что не смущает количеством строк в методе, или есть какая то ещё причина?

boolivar 9 окт 2014 в 18:17

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

MercurieVV 10 окт 2014 в 22:55

В общем все зависит от того чего именно вам нужно добиться.
Вариант с массивом подойдет если нужно генерировать последовательность программно — например мы хотим сделать редактор танца. Минус — если нам для каждого метода надо передавать специфические для него данные — например для парсинга пользовательской корзины нам нужен пользовательский ИД, для оформления накладно- ИД корзины.

timurv 9 окт 2014 в 16:06

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

Так плохо делать, модуль/класс должен делать что то одно. В вашем примере должно быть два класса.

1) Узнать информацию о поставщике;
2) Узнать информацию о лотах закупки;
3) Узнать информацию о аукционе закупки;
4) Узнать информацию о протоколах закупки;
5) Узнать информацию о последней дате изменения состояния закупки.

В вашем примере имеет смысл сделать разные классы для все 1-5 случаев скачивания и парсинга с одинаковым интерфейсом, и используя полиморфизм их вызывать в цикле.

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

Как то так с ошибками

class CalculateSomeShit
  ACTIONS = %i(
    SomeShit1
    SomeShit2
    SomeShit3
    SomeShit4
    SomeShit5
  )

  class SomeShit1
    def initialize(context)
      @context = context
    end

    def call
      @context[:shit1] = calculate(fetch)      
    end

    private

    def fetch
    end

    def calculate
    end
  end

  class SomeShit2
    def initialize(context)
      @context = context
    end

    def call
      @context[:shit2] = calculate(fetch(@context[:shit1]))      
    end

    private

    def fetch
    end

    def calculate
    end
  end  

  def call
    {}.tap do |context|
      ACTIONS.each do |action|
        context = action.new(context).call
      end      
    end
  end
end

FiresShadow 9 окт 2014 в 16:46

«класс должен делать что то одно»
На самом деле, не существует сущности, делающей только одно действие (если она состоит более чем из одной примитивной команды языка). Однако существуют сущности, делающие одну операцию НА НЕКОТОРОМ УРОВНЕ абстракции. На одном уровне абстракции, некоторый метод может производить два действия: скачивать и парсить, на другом уровне абстракции одну операцию — получить данные, на третьем уровне абстракции — миллион операций. Уровень с одной операцией более абстрактный, т.к. вам не важно, качаете вы страницы, используете API, или реплицируете базу данных. И название метода " Узнать информацию о лотах закупки" вполне говорит о том, на каком уровне абстракции он лежит. И на этом уровне абстракции у метода ОДНА область ответственности.
Этот пример был приведён для того, чтобы проиллюстрировать тот факт, что метод, внутри которого находится функция «Узнать информацию о лотах закупки» может раздуваться до больших размеров при раздутии доменной модели без возникновения запахов «стрельба дробью» и «расходящиеся модификации». А вот если вы разобьёте метод, в котором содержится метод «Узнать информацию о лотах закупки» на несколько методов, то возникнет запах «стрельба дробью» (относительно методов).
В статье я ничего не говорил о внутреннем устройстве этого метода и сам метод написан на псевдокоде, потому что частности его реализации не важны для демонстрации этого факта. «Узнать информацию о лотах закупки» может быть как НекоторыйПолиморфныйОбъект.Узнать(), так и просто вызов некоторой функции — это уже зависит от размера содержимого метода. Если там внутри пара строчек кода, то соблюдая KISS и high cohesion, раскидываем код по методам, а не по классам. А если внутри много кода, то выносим в отдельный класс. Вообще я предпочитаю метод и класс объединять термином «сущность».

«В вашем примере должно быть два класса.»
Конечно, класс Скачивальщик должен присутствовать, но он используется внутри функции «Узнать информацию о лотах закупки». Он мог бы и не использоваться (если бы мы обращались к API сайта). Но нас это не интересует. Нас интересует метод, псевдокод которого приведён в статье

anitspam 10 окт 2014 в 01:19

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

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

ApeCoder 10 окт 2014 в 02:16

А когда функция находится в боевом применении и работает, то причины на её изменение для следования каким-то стандартам должны быть очень вескими

Если у нас есть хорошие тесты или инструмент, который разобьет метод гарантированно корректно, то не такими уж и вескими

MacIn 10 окт 2014 в 02:34

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

symbix 10 окт 2014 в 05:02

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

kefirfromperm 14 окт 2014 в 05:04

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

Сложность понимания метода нелинейно возрастает в зависимости от его размера. Большой метод — это, однозначно, плохо. В редких случаях, альтернативы могут быть ещё хуже, но это не оправдывает большой метод.

FiresShadow 14 окт 2014 в 15:45

А вам не кажется, что вы сами себе противоречите?

kefirfromperm 14 окт 2014 в 16:53

Нет.

FiresShadow 15 окт 2014 в 15:16

В эксперименте во втором случае код разбивали на 2 листа или дописывали к первому листу ещё пол-листа?

kefirfromperm 15 окт 2014 в 16:38

Не помню

Bonart 19 окт 2014 в 21:13

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

FiresShadow 20 окт 2014 в 15:53

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

Для криптоалгоритма… необходимо… произвести декомпозицию в соответствии с этим описанием

Проблема как раз в том, что декомпозиция алгоритма в коде должна соответствовать декомпозиции алгоритма на едином языке доменной модели (Ubiquitous Language), а над единым языком программист не имеет власти. Это сказал wheercool в комментарии от 9 октября 2014 в 13:35. Потом на эту тему рассуждали довольно долго.

Для танца логично применить паттерн интерпретатор.

В комментарии от 9 октября 2014 в 18:21 boolivar с целью уменьшения размера метода предлагал завести массив из указателей на функции и в цикле этот массив обходить. Подводя итог обсуждения его предложения, можно сказать, что этот подход предоставляет некоторые преимущества, но за них нужно платить незначительным повышением сложности кода и незначительным уменьшением читабельности, и если не использовать предоставляемые преимущества, то незачем эту цену и платить. Вы предлагаете примерно тоже самое. Думаю, дело ещё и в том, что вы не понимаете, что такое паттерн проектирования. Паттерн — это типовое архитектурное решение типовой проблемы. Знание этого простого определения даёт понимание, когда нужно использовать тот или иной паттерн, а когда — нет. И критерий очень прост — когда есть проблема, то используем паттерн, который её решает, если нет такой проблемы — то, соответственно, не используем. Не нужно писать абстрактную фабрику, если нет сложного алгоритма создания объекта, тип которого зависит от фазы луны (или алгоритм создания зависит от фазы луны). По-хорошему, по мере нахождения не несущих полезной нагрузки абстрактных фабрик, нужно все лишние фабрики фабрик вычищать из кода, дабы не создавали лишней сложности. Точно также, если нет часто встречающейся, но подверженной изменениям задачи, а также если нет языка для интерпретации, предложения которого можно представить в виде абстрактных синтаксических
деревьев, то паттерн Интерпретатор использовать не нужно. Где, скажите пожалуйста, в аборигенском танце есть часто встречающая и подверженная изменениям операция? Присесть и покудахтать — это две совершенно разные операции, а не вариации одной и той же. Синтаксических деревьев тоже нет, алгоритм танца линеен. Или вы хотите выделить интерфейс ДействиеТанца (или вообще интерфейс ДействиеПрограммы, т.к. «присесть» может использоваться не только в танце) с единственным методом Интерпретировать, разбить код на сто-пятьсот маленьких классов (по количеству действий танца) и в функции СтанцеватьАборигенскийТанец создать массив из объектов-действийТанца, и в цикле их обойти? И что, в гигантском массиве копаться проще, чем в гигантской функции? А что вы будете делать, если в одну из функций вдруг понадобится передавать параметры? Перепишете пятьсот классов, потому что они наследуют общий интерфейс, или же будете передавать параметры через конструктор конкретного ДействияТанца? Чувствую, при таком подходе будет или много писанины, или потребуются навыки Шерлока Холмса, чтобы разобраться, что же тут происходит. Лично я никакой пользы не вижу от того, чтобы решать проблему паттерном, который для решение этой проблемы не предназначен. Паттерны проектирования — это, конечно круто, но не следует их использовать где попало. Тоже самое и с фасадом. Где в примере с закупкой сложная подсистема, с которой настолько сложно взаимодействовать, что нужно выносить взаимодействие с ней в отдельный объект? Неужели это функция распарситьЗакупку? Да там даже никаких объектов не создаётся (при условии что парсинг отдельной сущности уникален, но занимает несколько строчек кода. Если же парсинг большой и сложный — выносим его в отдельный класс), просто несколько функций одна за одной вызываются, пусть их количество может быть и велико (в зависимости от количества вложенных в Закупку сущностей на одном уровне абстракции — лотов, протоколов и пр.). К тому же, ну вынесите вы логику распарситьЗакупку в отдельный объект, назовёте его Фасадом, у Фасада будет всего один метод. Заглядываем в этот метод, а там внутри что? Неужели один в один прежнее содержание функции распарситьЗакупку, да ещё создание объекта добавилось? И зачем тогда всё это затевалось?

Bonart 20 окт 2014 в 16:44

Проблема как раз в том, что декомпозиция алгоритма в коде должна соответствовать декомпозиции алгоритма на едином языке доменной модели (Ubiquitous Language), а над единым языком программист не имеет власти.

Нет такой проблемы для криптоалгоритмов. Они разбиты на этапы и детализированы до уровня битов. Никаких проблем с аналогичной организацией структуры методов нет. Ссылка на DDD здесь нерелевантна.

Думаю, дело ещё и в том, что вы не понимаете, что такое паттерн проектирования.

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

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

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

Где в примере с закупкой сложная подсистема, с которой настолько сложно взаимодействовать, что нужно выносить взаимодействие с ней в отдельный объект

В постановке задачи. Указано, что источников исходных данных очень много — это само по себе говорит о дефекте проектирования и необходимости разбиения «главного класса» на несколько.

PS: «Чувствую» и «думаю» доказательствами не являются.

ApeCoder 21 окт 2014 в 05:30

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

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

3. Пример с разбором сайта не является примером большого метода — ТС пока не привел больше пяти строк и призвал мысленно дополнить другими строками.

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

Bonart 21 окт 2014 в 14:24

1. Дабы показать, что даже в такой чистой фикции длинный метод не нужен.
2. Про то и речь.
3. И это тоже — если начать мысленно дополнять, то на втором десятке источников класс надо будет разбивать… и снова длинные методы не нужны.
Ситуации бывают всякие, но лично мне пока ничего, кроме вышеупомянутых прототипов и еще генерации кода, не попадалось. Автору пока даже придумать такой случай не удалось.

Colwin 30 окт 2014 в 09:15

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

Думаете, почему многие проблемы более-менее решаются инженерными способами, но не имеют адекватных решений в научных трудах? Все дело в постановке задачи.

Зарегистрируйтесь на Хабре, чтобы оставить комментарий