Комментарии 181
Автор, понимаю, что это перевод, но не могли бы Вы добавить голосование в конец статьи кому какая версия кода больше нравится. Было бы интересно посмотреть результаты.
Левый вариант не мусорит в пространстве имен.
В правом варианте есть ошибка нейминга, функция box перетрёт класс box.
Конкретно в данном случае, больше нравится левый вариант как раз за линейность. Все линейные рецепты должны быть линейными, если им это позволяет сложность и функция не превышает ~сотню строк (24, если правки смотрят через ssh).
Избыточная декомпозиция как правило вредит, как раз по причинам постоянных прыжков. Недостаточная - сложнее модифицируется при меняющихся условиях задачи. Балансируем, ребят.
Я согласен с автором оригинальной статьи, что левый код читается лучше, т.к. фрагмент кода небольшой, все умещается на одном экране. Однако код в камне не высекается и имеет свойство со временем меняться. Если не меняется, то скорее всего этот код никому не нужен и читабельность его может быть какой угодно, т.к. никто его читать не будет. Если же код живой, то вариант слева со временем может вырасти в нечто нечитаемое строк на 1000, а то и более. Первоначальные авторы кода могут со временем уйти в другие проекты/компании etc. Приходят какие-нибудь стажеры. Или просто люди, которым надо по-быстрому запилить фичу здесь и сейчас. Код справа более устойчив к хаотичным правкам, его читабельность со временем будет деградировать медленнее.
"24, если правки смотрят через ssh". Подключился я тут давеча со свежайшего компьютера с pentium II и новехоньким ЭЛТ-монитором к серверу код пиццерии поправить ...
Вы таки не поверите, с чего иногда работают люди. Иногда это мобилка, иногда - КПК со стилусом, иногда - ноутбук-наладонник а ля gpd pocket. Иногда - пользуются тайловыми де, иногда - обнявши мачту яйцами заливают прошивку с наладонника в устройство на вершине мачты, и желательно проверить что ставится то что нужно (собранное на этом же устройстве).
И эти люди, опять таки, иногда - требуются в вашей заботе.
функция не превышает ~сотню строк (24, если правки смотрят через ssh).
какая связь между ssh и числом строк? при подключении с телефона может быть и меньше строк (а самое неприятное — строки куда у́же стандартных 80 символов), а при подключении со стационарного компьютера с 32" монитором там много строк (и это не предел, как минимум 43" используются достаточно часто)
Просто я обычно использую цифру 24 как стандарт.
Это всё эвристика. От неё, как правило хуже не будет. Напоминаю, что только ситхи всё возводят в абсолют, а баланс силы примерно посередине (или на уровне стандартов, используемых в ваших условиях). И стандартное окно терминала, открываемое в гуе без дополнительных манипуляций, примерно этому соответствует.
Я бы сказал что это очень зависит от проекта. Что если кроме пиццы вы готовите еще что-то? Вам придется дублировать код подогрева печи в каждом блюде. Так что нужно знать где и когда как писать, ане бездумно заворачивать все в отдельные функции по одной строчке.
Иногда, в том чтобы продублировать какую-нибудь фигню, нет ничего плохого. Особенно если дублируется какая-то мелочь небольшое количество раз во имя улучшения общей читаемости (линейности).
С этого всё и начинается - ничего страшного если я выделю вот эти пару строк и скопирую их вот сюда. Дальше - больше и поехали. Я считаю надо всегда очень хорошо подумать, прежде чем сделать копировать, вставить.
Только ситхи всё возводят в абсолют.
Идеального кода в реальной жизни таки нет и не будет, и чем больше ты стараешься "сделать всё идеальным по моим, или чьим-то ещё представлениям" - тем больше времени своей единственной жизни тратишь на ерунду. И когда от "дальше-больше", потраченного времени на решение проблем начинает тратиться больше чем на быстрые правки в т.ч. с копипастой - это некислый такой повод отрефакторить всё приложение или его часть, под изменившиеся вводные данные. Или нет, в зависимости от ситуации.
На C# я бы ещё и в отдельные классы вынес и внедрение зависимостей сделал
Если рассмотреть покрытие кода тестами, то на левый сложнее писать, так же больше вероятность конфликтов. В книге "Совершенный код" предлагается разбивать код на методы по 7-12 строк
Разбивать нужно не ради строк
Книгу нужно читать задумываясь почему это так, а не следовать бездумно
И что же там сложного в тестировании?
Как вы планируете для левого кода написать тест на то, правильно ли разогревается печь?
И при чём тут печь? Задача функции - делать вкусную пиццу. Как она там печку греет и использует ли её вообще совершенно не важно.
Тестирование бывает разного уровня. Если вы хотите только что-то типа интеграционного теста, то конечно без разницы, как греется печка, главное проверить, что пицца в коробке и готова.
Однако юнит-тестирование имеет свои плюсы.
Какие же?
Фиксация и проверка фрагментов бизнес логики.
Я с вами не согласен, бизнес логику на практике (а не в выдуманном примере) можно протестировать только интеграционным тестом, юнит тесты имеют смысл только для библиотечного кода, где бизнес логики нет от слова совсем.
Все те тесты, что мнят себе юнит и пытаются в бизнес логику, по факту тестируют моки. Удаляются пачками без сожалений, но самое главное (зависит от языка конечно) могут уродовать АПИ и усложняеть реализацию, только для того, что бы согласно идеологии, писать моки, без которых не получаются юнит тесты.
Ещё раз юнит тестами не надо тестировать бизнес логику они не для этого. Юнит тестами тестируют настоящую логику, математику/вычисления, универсальные алгоритмы и структуры данных, вот тут юнит тесты незаменимы. Юнит тестами тестируют код который может работать как есть в любой прикладной области.
Для тестирования бизнес логики пользуйтесь интеграционными тестами. Бизнес логика - это оксюморон, этими словами обычно именуется самая не логичная часть(и) кода. Правила, а чаще, просто желания людей, кодируются сразу во многих слоях приложения. Так происходит по многим причинам, девелоперы не понимают предметной области и/или языка бизнеса и/или бизнес сам не способен выразить свои хотелки понятными словами и что чаще бизнес не знает чего он хочет в конечном счёте и постоянно меняет правила и требует новой реализации уже позавчера.
Когда нужны моки? Ну когда написать фэйковые зависимости сильно сложнение чем применить моки и/или фэйки работают недостаточно быстро для теста, хотя я ни того ни другого пока не встречал.
Каюсь, я написал буквально тысячи бессмысленных и беспощадных юнит тестов бизнес логики с моками стабами и вот этим вот всем. Теперь почти все мои немногочисленные тесты это интеграционные тесты.
А плюсы в чём? Фиксацией реализации вы усложнили себе рефакторинг, не получив ничего взамен.
А вот тут не соглашусь. Фиксацией текущего состояния я помогаю себе найти, что сломалось при рефакторинге кода. Иначе - рефакторнул микро тех должок, на автомате, при пролистывании, и через несколько часов рефакторинга уже непонятно, почему не заводится. Мини юниты здорово спасают нервы на огромном проекте и уменьшают время рефакторинга, как бы странно это не казалось коллегам только начинающим работать в том же проекте.
Тесты, что фиксируют код, падают всегда, когда вы этот код меняете. Система контроля версий с задачей поиска изменившегося кода справляется куда лучше. А главное - делает это автоматически, надёжнее и гораздо быстрее.
У нас, например, 100% покрытие, включая ветвления. Это дисциплинирует, учит писать структурировано, учит писать только то, что нужно, и не писать то, что не нужно. Тест - это ещё и документ, объясняющий не очевидную логику. А спокойствие, которое испытываешь после масштабных рефакторингов и апдейтов просто бесценно.
Я с вами согласен, но вопрос был по сути не о том. Человек искренне не понимает, а как вообще такое надо тестировать.
@Hardcoin тест был бы, довольно прост. На вход подаем один из типичных заказов. На выходе проверяем соответствие размера, топинга, соуса заказу и опционально просто наличие сыра. Смотрим, что пица запеклась, как то порезана и не голая (в коробке). Степень запекания, цвет коробки, вид нарезки категорически игнорируем, а любое явное упоминание в коде теста печи и прочих инструментов, настоятельно требуем удаления на код ревью. Никаких моков, но если нужно пишем генератор заказов/кастомера, который тупо всегда возвращает один и тот же типичный заказ, и если нужно пишем официанта который только и умеет что взять в руки эту птицу.
Даже если позже мы вынесем код по нарезке, упаковке, то ничего в тестах менять не нужно. И даже, (берегись ерись!) для функции нарезки/упаковки я бы не писал теста т.к. этот код уже покрыт этим тестом.
этот код уже покрыт этим тестом.
Не покрыт, так как разнообразные случаи не рассмотрены. Вы же явно игнорируете степень запекания и вид нарезки. Постоянно пригоревшая пицца - не то, что ждут клиенты, возможно код-ревью придется делать в обход вас.
Да, действительно, у интеграционных тестов есть проблема: как протестировать все варианты параметров каждого из под-процессов. Получится, что интеграционных тестов надо запускать десятки/сотни, когда для тестирования какой-то конкретной функциональности можно было написать те же десятки/сотни тестов, но гораздо более простых, чем большые тесты, гоняющие весь процесс в сборе.
О подгоревшей пице мне скажут кастомеры (именно во множественном числе, суть уточнения в том, что одиночные подгорания меня не обеспокоят, у нас правило 10/10 юзеров/эвентов), я заведу баг и когда придет время его фиксить я напишу ещё один тест который будет воспроизводить условия при которых пица подгорает, таким образом я удостоверюсь, что тест полезный он бес фикса будет красным. Затем исправлю код и забуду об этом.
Вы можете не верить, но код покрытый интеграционными тестами тоже покрыт на ~85%, только это не спасает от багов почему то. Забавно то, что баги возникают в том числе и в том коде который покрыт от сюда и до забора.
Покрытие это не цель, особенно покрытие бессмысленно если оно достигается моками.
Мы все знаем, что зелёные тесты не значат, что в программе нет ошибок, при любом покрытии. Зачем же нам тогда тесты? Ну вам не знаю, а нам, что бы детектировать бизнес проблемы. Не любой баг это проблема. Ну и что что пица подгорает раз в пятилетку, у одного кастомера который заказывает голый корж? Из моего опыта, интеграционные тесты это самый экономичный способ мониторить проблемы бизнеса в коде. Когда то я тоже считал, что корректность кода должна быть пре выше всего, но это не всегда разумно.
Грустно быть вашим пользователем. Если ваше руководство это устраивает и оно платит деньги, никаких проблем. Но ведь это вопрос, не относящийся к тому, левый вариант хорош или правый.
Просто вы не проверяете даже очевидные случаи. Не те, что раз в пятилетку, а самые очевидные - не сгорит ли каждая пицца (а в коде нет остановки нагрева, возможно, что каждая сгорит). Или проверяете вручную. Но это не делает код слева лучше. Просто вам и так нормально.
Вы слишком идеализируете тесты. Если пицца горит из-за отстутсвия в коде остановки нагрева в определённом случае, а тест это проверяет, значит разработчик знает об этой ситуации, обработал её и в тестах, и в коде.
Чаще всего, разработчик вообще не подозревает, что нагрев надо останавливать. На это не написан тест. Соответственно, все тесты зелёные, а пицца на проде — горит.
Дело не в том, что бы все тесты были зелёными, а в том, насколько просто добиться работоспособности кода.
Можно, как я упомянул, вручную проверить несколько случаев, сгорела ли пицца. Конечно это возможно только в том случае, если разработчик или qa об этом подумал. Но как выяснить, на каком этапе проблема? В температуре, во времени, толщине теста или там более фундаментальная проблема? Для этого нужно проверить каждый кусок поэтапно (не обязательно автоматическими тестами). Найти нужный этап в длинной портянке, связанной десятком взаимозависимых переменных намного сложнее.
Разделение на функции - один из способов снижения связанности кода. Когда в нем пара десятков строк, как в примере, прочитать не сложно. Если в нём сотня строк, разделение на функции приносит заметный профит.
Изначально, был подход в основе которого были юнит тесты, что выразилось в краш рэйте 70%. Это значит 3 из 10 запусков нашего приложения заканчивались крашем. К этому моменту кодовой базе было более 6 лет. С момента смены акцента в тестах на интеграционные и авто / енд то енд прошло почти 5 лет, за это время краш рэйт стал 99,95 это значит только 1 из 2000 запусков заканчивается крашем + функционал значительно вырос. Я обсалютно убежден, что наши пользователи довольны сменой акцента наших тестов. Юниты остались, как и люди которые все ещё продолжают не понимать когда юниты действительно нужны. Забавно, то что это почему то те же самые люди которые топят за слоистые архитектуры, ну те которые дядя боб описывал, все эти контролёры, модели, одна функция не больше вашего любимого магического числа, инверсии здравого смысла под видом зависимостей и обажатели прыжков по кодо базе. Я лично не фанатик и не трогал такого вида код который работал, он для меня просто не существует, но когда мне приходилось чинить один и тот же код раз за разом, то я выпиливал все это вместе с юнит тестами.
Я не понимаю, как приложение может падать. Вероятно, ко мне оно приходит уже в более приличном виде, но мысль, что программист отправит в репозиторий падающий код меня удивляет. За ваш опыт спасибо, вероятно он сильно отличается и ситуация другая.
При чем тут магическое число не ясно, видимо вы хотите приписать мне то, что я не говорил.
У нас десктоп + эмбед на с++. Это значит любое не тривиальное приложение на плюсах падает без исключений, таков с++ с этим ничего не поделаешь.
Программист даже не догадывается, что новый код приводит к падениям, это нормально.
Магическое число к вам не имеет отношение, это отсылка к правилу где произвольным образом выбирается "правильная" длина функции в строках кода.
что выразилось в краш рэйте 70%. Это значит 3 из 10 запусков нашего приложения заканчивались крашем.
Почему вы называете краш рэйтом процент успешных запусков?
Мне это кажется неожиданным.
(И почему не «крэш-рейт»?)
Врагу не пожелаю писать код в проекте, состоящем из функций в 7-12 строк :D
По идее. Одна функция = 1 действие. Если функция делает два и более действия, то надо выделить эти действие в отдельные функции, ну мне так кажется
А как определить, сколько действий выполняет данный код, и самое главное - может ли количество действий в одном и том же коде различаться для разных задач/требований?
Например, вот эта функция печати решений квадратного уравнения - сколь действия она делает? На сколько более мелких функций её надо разбивать, и надо ли вообще разбивать?
function print_quadratic_solutions(a, b, c) {
const d = b * b - 4 * a * c;
if (d < 0) {
console.log("0 корней");
} else if (d == 0) {
console.log("1 корень", -b / (2 * a));
} else {
console.log("2 корня", -b - Math.sqrt(d) / (2 * a), -b + Math.sqrt(d) / (2 * a));
}
}Вроде логично разбить на 2 подфункции: решение и получение численных ответов, и печать, верно? А если следовать заветам Дядюшки Боба очень фанатично, то можно и до
такого дойти
function print_0_solutions() {
console.log("0 корней");
}
function print_1_solution(x) {
console.log("1 корень", x);
}
function print_2_solutions(x1, x2) {
console.log("2 корня", x1, x2);
}
function print_solutions(x) {
switch (x.length) {
case 0: {
print_0_solutions();
break;
}
case 1: {
print_1_solution(x[0]);
break;
}
case 2: {
print_2_solutions(x[0], x[1]);
break
}
}
}
function get_number_of_solutions(d) {
if (d < 0) {
return 0;
} else if (d == 0) {
return 1;
} else {
return 2;
}
}
function calculate_discriminant(a, b, c) {
return b * b - 4 * a * c;
}
function solve_quadratic(a, b, c) {
const d = calculate_discriminant(a, b, c);
const n = get_number_of_solutions(d);
switch (n) {
case 0:
return [];
case 1:
return [-b / (2 * a)];
case 2:
return [-b - Math.sqrt(d) / (2 * a), -b + Math.sqrt(d) / (2 * a)];
}
}
function print_quadratic_solutions(a, b, c) {
const solutions = solve_quadratic(a, b, c);
print_solutions(solutions);
}В 6 раз длиннее, зато каждая функция настолько мала, насколько возможно, ага.
А теперь представьте, что этот код размещён на HTML-странице, где есть поля ввода для a,b,c и кнопка для решения. Я бы вообще не стал делить этот код на функции, или даже отделять от получения значений из HTML-элементов, мне это кажется избыточным, я бы всё это поместил в единственный onсlick-обработчик.
И ведь никто не скажет как объективно лучше...
А потом может понадобится получение решение уравнение не только там. ну и в других местах.. От функций код не станет менее нечитабельным, любое IDE может прыгнуть к нужной функции от одного нажатие, тем более если функции поделены на действия, то тебе даже не надо знать её код. Все ясно по названию .
Хорошо, когда таких функций 5-7 вместо одной, это ещё можно удержать в голове.
Но когда функций становится 700 вместо 100, меньший кусок помещается в "кеш" и эффективность работы падает — только и делаешь, что прыгаешь между декларациями.
А потом может понадобится получение решение уравнение не только там. ну и в других местах.
Именно поэтому я написал "Вроде логично разбить на 2 подфункции: решение и получение численных ответов, и печать".
От функций код не станет менее нечитабельным
Опять-таки, всё дело в количестве, самостоятельности и возможности переиспользования этих функций.
Вот, в моём раздробленном примере выше:
calculate_discriminantполезна только для решения квадратных уравнений, больше эта формула нигде не используется - значит, функцию можно явно встроить в место вызова.get_number_of_solutions- тем более подходит только для квадратных уравнений, ведь она смотрит на тот самый дискриминант из предыдущего пункта. Её, конечно можно адаптировать чтоб она смотрела на коэффициенты, но это будет полное переписывание её самой, и тех, кто её вызывает, и всё равно для уравнений разной степени там будут отдельные блоки кода.print_0_solutions, print_1_solution, print_2_solutions- вы точно уверены, что в этих функциях-однострочниках есть хоть какая-то польза? Если вы завтра добавите поддержку уравнений вплоть до 4-й степени - вы напишете 5 таких однострочников? А если до 1000-й степени (не в общем виде, но это уже чисто алгебраический вопрос)? В какой момент вы решите, что надо вместо этих микрофункций использовать цикл по решениям и библиотеку, склоняющую слово "корень"? Но ведь это новое решение будет слишком сложным для квадратных уравнений, поэтому прямо сейчас пусть будет как есть (KISS)?
Дробление на функции - это как дробление чека в магазине: купить упаковку акварели отдельно от бананов - это одно (особенно учитывая что далеко не во всех магазинах есть сразу оба товара), но будете ли вы пробивать разными чеками макароны и хлеб, или тем более макароны разной формы? А ведь при подсчёте месячных трат именно на макароны, анализировать только "макаронные" чеки будет проще, чем выискивать подходящие товары из "общепродуктовых" чеков...
> И ведь никто не скажет как объективно лучше...
Можно использовать какие-то критерии и принципы вроде SRP
SRP (как и многие другие принципы разработки ПО) не объективен потому что отсутствует конкретный алгоритм, можно только спросить разных людей и получить разные мнения.
В моём раздробленном примере выше:
функции
print_0_solutions, print_1_solution, print_2_solutionsследуют SRP, но они малы до состояния замусориванияget_number_of_solutionsимеет более одной причины быть переписанной (замена аргумента с дискриминанта на коэффициенты уравнения, добавление поддержки уравнений 1,3,4 степени, добавление поддержки комплексных чисел), то есть она не следует SRP - как разбивать будете?
SRP (как и многие другие принципы разработки ПО) не объективен потому что отсутствует конкретный алгоритм, можно только спросить разных людей и получить разные мнения.
эм, это как? смысл в таких принципах же как раз в том что они должны давать достаточно четкий критерий. Иначе зачем их было формулировать?)
то есть она не следует SRP - как разбивать будете?
почему не следует? кто формулировал требования к функции, какая роль была у этих людей?
так по примеру можно заключить примерно, что:
был математик, который сформулировал для вас алгоритм решения
был дизайнер или там бизнес-аналитик, который указал в каком формате над выводить решение (подписи вида "n корней корень")
с-но по SRP у вас будет два модуля, один отвечает за требования математика, другой - аналитика.
первая ф-я будет возвращать структуру с решением, вторая - получать на вход эту структуру и выводить ее в заданном формате.
естественно, если процесс формирования требований был каким-то другим, то и разбиение будет иное. в этом фишка SRP - он смотрит не на сам код, а на предметную область и организацию процессов.
смысл в таких принципах же как раз в том что они должны давать достаточно четкий критерий.
Пример с SRP на словах, считаем действия, выполняемые функцией. Два варианта:
Функция получает на вход коэффициенты a,b,c квадратного уравнения, и возвращает массив с уникальными вещественными решениями.
Функция получает на вход коэффициенты a,b,c квадратного уравнения, считает дискриминант, определяет количество уникальных вещественных корней, считает эти корни, и наконец возвращает эти корни в массиве.
В первом случае она как будто бы делает одно действие, а во втором - аж четыре (!), хотя алгоритм абсолютно один и тот же. Но в первом случае "на слух" SRP соблюдён, а во втором надо разбивать на 3 части (см solve_quadratic ). А действительно ли это нужно? Да, в функции выполняется более одного шага, и отдельные микрофункции действительно будет проще обмазать тестами, но зачем? Решение квадратных уравнений - элементарный алгоритм, который буквально проходят в школе, да ещё и потом решают десятки если не сотни этих уравнений. Тем не менее, если фанатично следовать SRP - да, нужно разбивать, потому что Дядюшка превыше здравого смысла.
естественно, если процесс формирования требований был каким-то другим, то и разбиение будет иное. в этом фишка SRP - он смотрит не на сам код, а на предметную область и организацию процессов.
Насколько мне известно, это как раз и называется "не объективный". Объективные метрики могут быть измерены по чётко известной процедуре. Например, вычислительная сложность алгоритмов - есть и теоретический подход со всеми этими прекрасными O(n log n), и можно наделать практических тестов. А как измерить единственность ответственности?
А если вернуться к предыдущей цитате, то ещё забавнее получается. В текущий момент времени мне не надо иметь решение уравнения отдельно от печати - то есть YAGNI и KISS говорят, что дробить не надо (потому что больше функций - это не нужно и сложнее), а SRP - что надо. В итоге счёт 2:1 не в пользу SRP, бууу, принцип-неудачник?
В первом случае она как будто бы делает одно действие, а во втором - аж четыре (!),
Эм, ну и что? С точки зрения СРП вообще не важно, сколько действий делает ваша ф-я. Не важно в принципе, что она делает и как реализована. Может и ничего не делает - и при этом по СРП ее надо будет вынести, а может экспедицию на марс отправляет - и при этом по СРП ее выносить не надо будет.
Насколько мне известно, это как раз и называется "не объективный".
Почему не объективный? Имея конкретную ситуацию у нас есть конкретный ответ. Понятное дело, что он не математически строгий (тут математической строгости принципиально не добиться). Но если вы зададите вопрос, там, нескольким десяткам программистов - то почти наверняка они все дадут один и тот же ответ. Этого с практической точки зрения вполне достаточно.
А как измерить единственность ответственности?
Так я же вам указал, как. Смотрите сколько ролей отвечают за требования к данному модулю. Это и есть количество ответственностей. Если >1 - по СРП надо дробить модуль.
В текущий момент времени мне не надо иметь решение уравнения отдельно от печати
Тогда у вас, почти наверняка, не будет отдельно математика и отдельно дизайнера отвечающего за вывод.
Логика-то за СРП простая - если в вашем процессе разработки есть две отдельные роли для чего-то там, то это разделение _важно_. И раз оно важно - то вполне логично его зафиксировтаь на уровне архитектуры вашего кода.
Вот, например, может так статься, что у вас будет не один математик, а дискриминантописатель и дискриминантоюзер - один пишет алгоритмы для вычисления дискриминантов, другой пишет алгоритм для вычисления корней через эти дискриминанты. И вот если оно именно так, две отдельные роли (а не просто два взаимозаменяемых математика которые делают общую работу) - то значит в рамках ваших процессов и в рамках вашей предметной области такое разделение _существенно_. Вполне разумно и в рамках кода это разделение поддержать. Чтобы если вдруг вам придет новый алгоритм расчета дискриминанта, то модуль который этот дискриминант использует остался незатронутым.
И думать о том, сколько там в вашей ф-и действий, не нужно, с точки зрения СРП это все лишнее совершенно.
Эээ...
Вот у нас есть проект программы, решающей уравнения. Функция решения квадратного уравнения написана в соответствии с алгоритмом от штатного математика, он единственный "ответственный". В целом проект насколько наSOLIDоленный, что сам Мартин бы расплакался от счастья (если бы знал про него, конечно).
Прекрасным сентябрьским днём в проект наняли отдельного дискриминантооптимизатора, чтоб ускорить решение, но через два дня уволили (потому что не надо приходить пьяным и материть одного из верхних менеджеров, он обижается). Код решения за всё это время не был изменён, просто не успели.
Правильно ли я вас понимаю, что сначала код удовлетворял SRP (а значит и SOLID в целом), затем нет, а затем снова да, несмотря на отсутствие изменений?
Правильно ли я вас понимаю, что сначала код удовлетворял SRP (а значит и SOLID в целом), затем нет, а затем снова да, несмотря на отсутствие изменений?
it depends.
Если вы внимательно читали то, что я пишу, то должны были видеть, что я говорил о ролях - один человек может шарить несколько ролей, как и на одной роли может работать несколько человек. Поэтому неважно, как и кого вы увольняете или нанимаете.
Безусловно, потребности проекта могут меняться, но я с трудом представляю ситуацию, в которой вот вы осознали, что у вас действительно есть роль дискриминантооптимизитора а потом бац! и на следующий день оказывается, что уже не нужна. Подобного рода вещи - это достаточно длительный процесс.
А так, в целом, да - код сначала удовлетворял СРП, а потом нет. Но вы неверно указываете на то, что изменений не было - они были. Изменения в понимании того, что этот код должен выражать, каков характер описываемых кодом процессов.
В реальности, если у вас есть некая информационная система, обслуживающая некие бизнес-процессы, то при изменении ролевой модели этих процессов вам сразу задача на доработку придет и так в 9 случаях из 10.
И, да, это близкий к граничному пример, но он возникает как раз из-за того что СРП достаточно точен. Любой принцип, который дает точную оценку, позволит построить такой пример, потому что мы хотим бинарного ответа, с-но если принцип точен то он на своей "границе" будет давать противоположные ответы в сколь угодно близких ситуациях. И "перещелкивать" ответы при колебании около этой границы.
Представьте, что у нас принцип вида "не больше N строк" - вполне четкий, формально строгий. Вот у нас ф-я N строк, немного форматирование у нее поменяли - теперь N+1. Поменяли обратно - опять N! И вот у вас функция, которая ни как не меняется кроме форматирования, но тут ее надо декомпозировать - а там не надо.
То есть, сначала вы утверждаете
если вы зададите вопрос, там, нескольким десяткам программистов — то почти наверняка они все дадут один и тот же ответ
А потом оказывается, что ответ зависит от процессов в организации и организационной структуры. Тогда, во-первых, выше надо спрашивать не программистов (откуда им знать про роли, им таски прилетают — они кодят), во-вторых, сколько организаций, столько и ответов (в пределе).
То есть, сначала вы утверждаете
А потом оказывается, что ответ зависит от процессов в организации и организационной структуры.
А где тут противоречие? Архитектурные вопросы ведь всегда принимаются с учетом этих (и не только этих, а даже с учетом рабочей ситуации в моменте) факторов. Собственно, работа разработчика по большей части именно в их учете и состоит, за это ему деньги и платят. Иначе можно просто было бы посадить вместо этого разработчика обезьянку, дать банан - и она прекрасно справилась бы с работой.
Тогда, во-первых, выше надо спрашивать не программистов (откуда им знать про роли, им таски прилетают — они кодят)
Если разработчик не понимает, откуда, как к нему прилетают задачи, и почему они именно такие - то либо это плохой разработчик, либо плохо организованный процесс разработки. В обоих случаях надеяться на то, что архитектура может получиться качественной, не стоит.
во-вторых, сколько организаций, столько и ответов (в пределе).
Все верно. Ну так и решение по архитектурным вопросам в каждой организации должно быть свое, с учетом особенностей этой организации. Архитектура, подходящая для одной организации, для другой уже не подойдет. Архитектура, подходящая для одной предметной области, для другой не подойдет. Архитектура, которая реализуется в рамках одного процесса разработки - для другого не подойдет. Архитектура, подходящая для одной команды - для другой не подойдет. Архитектура, которая решает важную проблему в данный момент времени - в другой решать ее не будет. Ну и так бесконечно можно продолжать.
И как получается, что задав вопрос десятку программистов (из разных организаций), вы ожидаете одинаковый ответ?
Причём, вопрос не абстрактный, типа "надо ли следовать SRP?", где они могут сойтись во мнениях, а максимально конкретный: "нарушает ли SRP вот этот конкретный код?"
И как получается, что задав вопрос десятку программистов (из разных организаций), вы ожидаете одинаковый ответ?
так мы же спрашиваем в контексте конкретной ситуации. это часть задачи. нельзя обсуждать архитектурные вопросы в отрыве от подобных факторов.
"нарушает ли SRP вот этот конкретный код?"
Такого вопроса не бывает. Ну, точнее, на него невозможно ответить, да и смысла в нем нет, код не может нарушать или не нарушать SRP сам по себе. Потому что SRP - это архитектурный принцип, а архитектурные решения не могут быть корректными или нет сами по себе. Абсолютно любой принцип написания кода может (и должен) нарушаться в определенных ситуациях, поэтому обсуждать такие вещи не фиксируя конкретную ситуацию и ее контекст - невозможно.
Правильный вопрос: "нарушает ли SRP данный код, если этот код предназначен для ..., разрабатывается командой ... в ходе процессов вида ...." - и дальше уточняем внешние факторы.
Вы же не можете глядя на код сказать правильный этот код или нет, если не знаете, что он должен делать? Ну вот и тут так же.
SRP не про действия, а про ответственность. Ваша функция отвечает за решение квадратного уравнения и за вывод решения на экран. Поэтому нужны две разных функции.
А вычисление дискриминанта вообще не требуется. Функция вычисляет его для себя.Поэтому для удовлетворения SRP не требуется выделять это вычисление в отдельную функцию. Более того, его вообще необязательно вычислять (при условии, что существует другой, достаточно эффективный метод решения квадратных уравнений).
Соглашусь, что "не объективность" присутствует. Более того, даже один и тот же программист может разбить задачу на функции по-разному для разных языков программирования.
ваш линейный вариант не асинхронен. Программа блокируется пока пицца не приготовится. Правый вариант типичные асинхронные вызовы с коллбэком
Вероятно, этот код должен получать печь в качестве параметра. Её предоставление —задача вызывающей стороны.
Получать печь в качестве параметра тоже не очень удачная идея, тут синглтон прямо напрашивается
я бы оформил вот так:
func createPizza(order *Order) *Pizza
{ // Готовим пиццу
pizza := &Pizza{Base: order.Size, Sauce: order.Sauce, Cheese: “Mozzarella”}
// Добавляем топпинги
if order.kind == “Veg” pizza.Toppings = vegToppings
else if order.kind == “Meat” pizza.Toppings = meatToppings
oven := oven.New()
if oven.Temp != cookingTemp
for (oven.Temp < cookingTemp)
{ // Разогреваем печь
time.Sleep(checkOvenInterval)
oven.Temp = getOvenTemp(oven)
}
if !pizza.Baked
{ // Печём пиццу
oven.Insert(pizza)
time.Sleep(cookTime)
oven.Remove(pizza)
pizza.Baked = true
}
// Кладём в коробку и нарезаем на куски
box := box.New()
pizza.Boxed = box.PutIn(pizza)
pizza.Sliced = box.SlicePizza(order.Size)
pizza.Ready = box.Close()
return pizza
}Это какой-то Си подобный язык? Меня смущает строка box := box.New()тут присваивание как в Паскале, хотя следом в строках ниже - как в Си.
это Go, := - объявление переменной с присвоением, без явного указания типа
https://go.dev/ref/spec#Short_variable_declarations
Ну, для сферической пиццы в вакууме код подходит. Но если приблизить его к реальности — то сыр закончился, то печь неисправна и не греется или температура гораздо выше нормы, то коробки остались только маленькие, а пицца большая, то клиент попросил порезать не на шесть кусков, а не семь… В результате единая функция распухнет и будет обмазана таким количеством условий, что без стакангенса не разберёшься.
В декомпозированной функции это будет ещё хуже. Будут по всему колхозу передаваться многочисленные флаги, что пошло не так на предыдущих этапах. Если, конечно, не принять волевое решение уничтожать любую пиццу, при готовке которой откинулся эксцепшен или что там у неё.
Обрисованная проблема-то фундаментальная – реальный процесс может быть не так линеен, как простейшая модель – а не связанная с оформлением кода.
А зачем передавать флаг - "печь неисправна"? Это уже означает что всё, процесс завершен, полуготовую пиццу придется выбросить, сырое тесто вы ведь не повезете клиенту, как и угольки вместо пиццы. Это и есть критикал эксепшен, который должен прервать весь процесс. А что касается всяких особых пожеланий при готовке - их можно передавать функциям в объекте (в структуре, если мы не в объектном стиле пишем), эта информация все равно должна где-то храниться и обрабатываться, глобальные переменные нифига не лучше для этого приспособлены. Эксепшены, которые можно разрешить - типа "есть только маленькие коробки - а пицца большая - при согласии клиента, разложим ее в несколько коробок" так-же можно в отдельных функциях обрабатывать.
Будут по всему колхозу передаваться многочисленные флаги, что пошло не так на предыдущих этапах
Это если вы пытаетесь писать код в левом стиле, но в отдельных функциях. Не надо так. И флаги не надо. Если у вас печь не работает, вам не нужен такой флаг при упаковке пиццы в коробку, вам вообще пиццу в коробку упаковывать не надо, она же не готова.
Может клиенту сырая пицца - лучше, чем никакой?
У вас такой максималистский подход, что в одном месте сломалось - значит всю работу останавливаем. Не везде он приемлем, и в управлении технологическим процессом - точно нет.
Если у вас реальная пиццерия с тысячью заказов, тысячью полуфабрикатов и сломавшейся печью, то видятся, например, такие альтернативы:
выкинуть все полуфабрикаты, аннулировать все заказы (это то, что предлагаете вы), попасть при этом на себестоимость и на штрафы от наиболее недовольных клиентов;
предложить клиентам сырую пиццу за полцены в надежде, что хотя бы кто-то согласится;
отправить сырую пиццу в коробках в другую фирму, которая согласилась допечь, и далее по клиентам.
Вам хоть раз доставляли/приносили в ресторане полуготовую еду вместо заказанной? Никто так не делает, но вы в своем бизнесе можете быть первым, если хотите.
А по поводу последнего пункта - если у вас контракт с другой фирмой есть, то конечно - пишете другой бизнес-процесс (собираете из готовых функций), где вместо печи - поездка в другую фирму и исполняете его. Но у вас должны быть готовые функции. Встроить подобную логику в линейную лапшу будет крайне проблематично, не правда ли?
Вам хоть раз доставляли/приносили в ресторане полуготовую еду вместо заказанной?
Именно единственная печка, как в вашем примере, в ресторанах ломается очень редко, поэтому с полуготовой едой не сталкивался (хотя предпочёл бы недопечёную пиццу никакой в ряде сценариев). Но другие согласованные отклонения от рецептуры бывали.
А по поводу последнего пункта - если у вас контракт с другой фирмой есть, то конечно - пишете другой бизнес-процесс (собираете из готовых функций), где вместо печи - поездка в другую фирму и исполняете его. Но у вас должны быть готовые функции. Встроить подобную логику в линейную лапшу будет крайне проблематично, не правда ли?
Предусмотрительный программист реализует альтернативные пути сразу. И именно поэтому не будет линейных последовательностей вызовов.
В конечном итоге в такой программе структурно образуется что-нибудь вроде цикла обхода состояний конечного автомата (если программист понимает, что он делает), или просто лапша (если не очень).
Недопеченная пица.. ну не знаю, корочка - внутри сырое тесто, не до конца обработанные термически продукты, можно просто отравиться.. Может вам и нормально но ни один уважающий себя бизнес такое клиенту не повезет.
Предусмотрительный программист реализует альтернативные пути сразу.
Оверинжиниринг не всегда приветствуется.
И именно поэтому не будет линейных последовательностей вызовов.
Тут мы с вами достигли согласия, кажется. Линейный код слева не очень подходит.
Никто так не делает, но вы в своем бизнесе можете быть первым, если хотите.
В моём бизнесе неполные технологические процессы – обычная практика жизни. Причин, препятствующих выполнению отдельных операций, очень много, а условный полуфабрикат – дорогой, не разбросаешься.
Поэтому логически каждая операция имеет, как практический минимум, три возможных результата, которые необходимо обрабатывать: операция выполнилась успешно; операция выполнилась неуспешно; во время операции возникло внешнее событие, отменяющее её выполнение. В нашем примере это будут, например: пицца испеклась; пицца постояла в печке, но не испеклась; в печку вместе с пиццей забрался кот.
Не ясно как себя поведут компоненты при попытке второй раз приготовить, гарантировать при этом БЕЗОПАСНОСТЬ продукта никто не возьмётся. И во вторых, чисто логистически разрабатывать и применить систему решений что-делать-дальше-если-.... дороже чем изготовление одной единицы продукта. А ещё нужна система датчиков чтобы ПОНЯТЬ ЧТО С ПИЦЦЕЙ НЕ ТАК, чтобы система принятия решений смогла принять решение. Проработать все варианты возможные, отладить это всё и протестировать.... тоже желательно во всех возможных вариантах, а не на производстве уже. Итого, система автоматической "доготовки" пиццы просто разорит предприятие. Проще выкинуть неудачный экземпляр, даже если это будет одна из сотни.
И кстати, один только факт что на выходе есть недоготовленные изделия может нести репутационные риски с последующим банкротством.
Присоединяюсь: в голосовалке нет варианта "ни один не нравится". Какой-то винегрет из ООП и процедурного программирования
Конкретно мне кажутся нелогичными такие места, как например
oven.Temp = getOvenTemp(oven)
Если печь у нас объект, то у неё должен быть метод oven.GetTemp(), возвращающий температуру. Странно узнавать состояние объекта с помощью сторонней функции. Также в моём понимании процесса у печи должен быть блокирующий метод oven.Heat(Temp), нагревающий печь до нужной температуры и только потом завершающийся, и асинхронный метод oven.SetTemp(Temp), задающий новую уставку температуры и завершающийся немедленно.
getOvenTemp(oven) тестировать легче, чем oven.GetTemp().
Но сама по себе функция имеет плохое имя и дизайн, тут согласен)
Хм...
Вот у нас есть объект oven. У этого объекта есть свойство oven.Temp,где хранится его температура. Однако чтобы это свойство имело актуальное значение, необходимо записать это значение в него вручную (oven.Temp = getOvenTemp(oven)), вызвав некую функцию getOvenTemp(), в которую в качестве параметра передаётся сам объект oven. И здесь у меня возникает несколько вопросов именно с программистской точки зрения:
раз уж мы передали в функцию
getOvenTemp()сам объектoven, почему эта функция не может сама установить этому объекту значение свойстваoven.Temp? Почему мы должны делать это вручную?Как это реализовано технически? Откуда функция
getOvenTemp()берёт значение температуры? Если из самого объектаoven, то почему мы не можем сделать то же самое без вызова этой функции? Если откуда-то извне, то как так получилось, что свойство объекта определяется какими-то внешними условиями?
Я не считаю себя профессиональным программистом, но даже на мой непрофессиональный взгляд это не код, а какая-то каша. ИМХО нельзя выставлять такое в роли образца для подражания. Впрочем я допускаю, что такое моё мнение - как раз следствие моего непрофессионализма. В таком случае буду рад разъяснениям, почему в обсуждаемом коде всё сделано правильно.
почему эта функция не может сама установить этому объекту значение свойства oven
Потому что функция хочет быть чистой. Чистые функции предсказуемы и крайне надёжны. Но от этого эпизодически страдает производительночть.
Если из самого объекта oven, то почему мы не можем сделать то же самое без вызова этой функции
Потому что это модификация состояния, что не всегда требуется.
Де факто, код справа переписан в функциональном стиле (хотя и не очень удачно), что позволяет получить определенные преимущества, особенно в контексте асинхронной/параллельной обработки пиццы.
Однако чтобы понять суть этих преимуществ, нужно хотя бы поверхностно окунуться в функциональные языки, на этом конкретном примере продемонстрировать их не получится.
Чистые функции так и норовят вызвать ООМ. Очень надёжно, да.
На моей памяти такого никогда не случалось (я имею ввиду, чтобы в погоне за чистотой, случился ООМ).
Написание кода в функциональном стиле - это хороший подход в очень многих ситуациях, я крайне его рекомендую и всячески пропагандирую. В более или менее приличных языках, имеющих хоть какую-то внятную (не обязательно хорошую) систему типов, компилятор способен оптимизировать оверхед.
Тем не менее функциональные языки мне не нравятся, на них жутко неудобно писать обработку ошибок и просто поразительное количество костылей нужно, если хочется повозиться с состоянием.
Поэтому мой совет - во всём нужно знать меру) Второй совет - используйте языки, позволяющие смешивать императивщину и функциональщину и будете счастливы)
Пример из моей практики: хеш-таблица на 1М записей. Надо добавить ещё одну запись - изволь скопировать несколько десятков мегабайт и надеяться, что в памяти есть непрерывная область достаточного размера. Про нейронки с гигабайтными моделями, которые даже в однократном экземпляре не всегда влезают в память, я даже не заикаюсь.
Тот гений, что это придумал - просто безумный фанатик)
Так надо иммутабельный хеш юзать
Это и есть иммутабельный хеш. Вы возможно имеете ввиду какой-нибудь btree, но у него асимптотика чтения сильно хуже.
>Это и есть иммутабельный хеш.
Тогда зачем он десятки мегабайт копирует при вставке?
Возможно потому, что не может изменить?
А смысл такого "хеша", если у него сложность добавления элемента O(N)? Чтобы вставить 1M элементов, нужно 1T времени.
Это самописный контейнер или из какой-то библиотеки?
Обычно иммутабельные хеши делают на hamt, и его целиком копировать не требуется.
То есть дерево с соответствующей асимптотикой. Ещё и не самобалансирующееся.
worst O(log(n)) с основанием 32-64 (т.е. константа с точки зрения практических задач) и amortized O(1), если перестраивать корневое дерево - это хреновая асимптотика? Ну у вас, батенька, и запросы, конечно.
Асимптотика чтения у хеш-таблицы всё же О(1), а у сбалансированного дерева O(log). Тут же даже не сбалансированное дерево, а это значит может быть 6 хопов вместо 1 даже на небольших объёмах.
она в среднем O(1), но если хеши будут совпадать, то затраты на поиск пропорциональны тому, сколько ключей попалось на этот хеш. Аналогично с hamt - 6 хопов там получится только тогда, когда для ключей совпали первые 25 бит (50 при 64битной реализации) их хешей. Если hamt мутабельная то мы к тому же можем перестраивать всю структуру по аналогии с хештаблицей, при чем можно делать это лениво - в итоге при среднем O(1) (так же как у обычных хештаблиц) worst case будет уже O(log(n)) в отличии от O(n) у хештаблиц, т.е. асимптотика даже лучше (но там есть свои нюансы офк). в иммутабельном случае перестраивать дерево не выйдет, да (точнее формально можно, но это деграднет перформанс операций кроме поиска и потребление памяти). Поэтому O(log(n)), которое на практике слабоотличимо от константы изза очень высокого бренч фактора, емнип поиск в иммутабельных hamt по перформансу по итогу где-то на 20-40% отличается от поиска в хештаблицах. Ну и копировать десятки мегабайт при добавлении не надо, офкос.
А кто реализовал хэш так через жопу? Обычно коллизии разрешают через, например, связные списки, так что в данном бакете просто просядет производительность, но десятки мегабайт перемещать не надо.
Очень интересный функциональный стиль, когда почти все функции мутируют подаваемые на вход данные вместо создания новых...
Да и выполняться это должно в строго фиксированном порядке (если, конечно, вы не хотите иметь сырые овощи поверх коробки, или разогревать печь уже после "выпекания"), что подразумевает сайд-эффекты / монады, которые... ах да, которые позволяют в функциональщину добавить императивщину.
Конкретно здесь, в примере, оба варианта плохи, но я бы предпочел увидеть скорее правый, чем левый.
Левый вариант нарушает, как мне кажется, базовое правило декомпозиции - все делается в одном месте, вместо того, чтобы иметь отдельные маленькие, понятные и тестируемые функции.
Это, кстати, можно видеть и в правом варианте (bake создает oven - почему?, зачем?), но в куда меньшей степени.
Для меня правый читается сложнее
... в первый раз
Но, если я в ближайшее время продолжу работать с этим кодом, то я буду приблизительно помнить ответы на перечисленные выше и любые другие вопросы по поводу функций.
"А если кто-то поменяет код, и твои ответы устареют?" - Я либо на этапе код ревью увижу изменения, либо и при работе с левым кодом ответы устареют, и я столкнусь ровно с такой же проблемой
В статье от Google высказана правильная мысль "Avoid mixing different abstraction layers into a single function", только их пример неправильно ей следует.
Действительно, не должно быть смешивания низкоуровневой логики и высокоуровневых абстракций. Из этого например следует, что одна публичная функция не должна вызывать другую публичную функцию, они обе должны вызывать приватную. Но следует так же и избегать большого количества промежуточных уровней абстракции. Следует стремиться к 2-3 уровням — высокоуровневое название публичного метода, высокоуровневые шаги для выполнения действия в виде вызовов приватных методов, технические детали реализации шагов. Во многих случаях высокоуровневый шаг всего один — вызвать реализацию действия, и его можно не указывать.
В примере с пиццей их несколько. Если мы хотим сделать максимально независимые шаги, которые можно переиспользовать в разных методах, то надо делать так.
func createPizzaInBox(order *Order) *Box {
pizza = createNewPizza();
addTopings(pizza, order);
if !pizza.Baked {
oven := getOvenDevice();
heatUpOven(oven);
bakePizza(oven, pizza);
}
box := takeNewBox();
packPizza(pizza, box);
slicePizzaInBox(box, order);
closeBox(box);
markAsReady(box.pizza);
return box;
}Все шаги на верхнем уровне, представляют собой только вызовы методов и их условия. Условия выполнения шагов, связанные с бизнес-логикой, надо тоже писать на верхнем уровне, а не прятать в детали реализации.
Если мы хотим сделать на верхнем уровне более крупные шаги, то можно сделать так.
func createPizzaInBox(order *Order) *Box {
pizza = createNewPizza(order)
if !pizza.Baked {
bakePizza(pizza)
}
box := packPizza(pizza)
return box
}Но тогда в этих методах надо сразу указывать детали реализации, не разбивая дальше. Иначе, как правильно заметил автор статьи, программисту, который не писал этот код, будет сложнее разбираться, как он работает, надо будет держать в голове контексты нескольких мелких функций и перемещаться между ними туда-сюда.
func bakePizza(pizza *Pizza)
{
oven := getOven()
if oven.Temp != settings.cookingTemp {
for (oven.Temp < cookingTemp) {
time.Sleep(settings.checkOvenInterval)
oven.Temp = getOvenTemp(oven)
}
}
oven.Insert(pizza)
time.Sleep(settings.cookTime)
oven.Remove(pizza)
pizza.Baked = true
}Напрашивается мысль, что прогрев печи все-таки лучше вынести в отдельный метод. Если поместить все такие методы в этот же класс, то появится виртуальная сущность "Группа методов, связанных с печью". Это как раз признак того, что класс перегружен, и правильнее вынести этот код в отдельный компонент.
func bakePizza(pizza *Pizza)
{
oven := getOven()
oven.heatUp(settings.cookingTemp);
oven.bakePizza(pizza);
pizza.Baked = true
}Тут тоже можно сказать, что другому программисту будет сложнее разбираться, но в данном случае детали реализации находятся в отдельном компоненте. Его проще изучать, он может использоваться и в другом коде, а также программист может решить "А, это печка, мне точно не нужны эти детали". Что нельзя сделать если это оформлено как детали приготовления пиццы, которые программисту как раз и нужны.
одна публичная функция не должна вызывать другую публичную функцию, они обе должны вызывать приватную
В чём смысл? Если у нас есть, например, сахарная Window::Display(), которая выводит окно в центр экрана и гарантирует, что оно будет отображаться, что мешает в её имплементации вызвать публичные Window::Show() и Window::Move()? Обязательно городить Window::ShowImpl()/Window::MoveImpl()?
Ну представьте, что во всех 3 функциях вызывается какое-нибудь логирование в начале и в конце. Для Display() нам надо сделать работу Show() и Move(), но залогировать только "Display begin / Display end". Это относится к любой дополнительной работе, которую нежелательно повторять или вкладывать — начало/коммит транзакции, установка/снятие мьютексов, подсчет количества вызовов отдельно для каждой функции и т.д.
Исключения бывают везде, но для поддерживаемости лучше начинать с такого подхода, и менять его если есть причины.
Ну представьте, что во всех 3 функциях вызывается какое-нибудь логирование в начале и в конце. Для Display() нам надо сделать работу Show() и Move(), но залогировать только "Display begin / Display end".
По-моему, это логическая ошибка «бесконечного регресса»: когда объяснение неявно ссылается на само себя.
Если нормально относиться к тому, что публичная функция может вызывать публичную функцию, откуда возьмётся странное требование Для Display() нам надо сделать работу Show() и Move(), но залогировать только "Display begin / Display end"? В логах будут «скобки» и «скоупы», это совершенно нормально. Я такие логи разбирал, ничего страшного в них нет. Ненормальным это кажется, только если взять за аксиому изначальное правило одна публичная функция не должна вызывать другую публичную функцию.
Это относится к любой дополнительной работе, которую нежелательно повторять или вкладывать — начало/коммит транзакции
А как транзакционность связана с обсуждаемым правилом? По-моему, это частный случай проектирования, особенные функции, и из них нельзя вывести универсальное правило, связанное с видимостью.
откуда возьмётся странное требование
От бизнеса или от техлида.
Я такие логи разбирал, ничего страшного в них нет.
Я не сказал, что в них есть что-то страшное. Я сказал, что бывают ситуации, когда надо логировать внешние вызовы и не логировать внутренние. Логи бывают не только в файлы для программиста, но и в базу для бизнеса. И если там будет в 2 раза больше каких-нибудь записей о платежах чем фактически, вряд ли его это устроит.
если взять за аксиому изначальное правило
Это не аксиома, а практическое улучшение поддерживаемости кода. Поддерживаемость подразумевает адаптацию кода к изменяющимся требованиям. Сегодня не надо было логировать, завтра надо. Сегодня не было транзакций, завтра появились. Ну и упомянутое смешение уровней абстракций, что усложняет понимание для того, кто не писал этот код. Собственно, появление новых требований как раз и проявляет разницу уровней абстракции. Если не хотите улучшать поддерживаемость, это дело ваше.
А как транзакционность связана с обсуждаемым правилом?
Так, что это пример дополнительной работы, которую нежелательно повторять или вкладывать, из-за которой лучше следовать этому правилу.
Или вот такой пример.
Display() {
Show();
Move();
}
Show() {
this.ensureScreenNotNull();
data = this.screen.getData();
...
}
Move() {
this.ensureScreenNotNull();
data = this.screen.getData();
...
}
ensureScreenNotNull() {
if (this.screen === null) {
this.screen = System.getScreen();
}
}В вашем варианте при вызове Display() функция ensureScreenNotNull() вызывается 2 раза, хотя достаточно 1. В каком-то другом коде в такой функции может быть тяжелая инициализация. Которая могла понадобиться не сразу, а через некоторое время после написания первоначального кода. Чтобы избежать лишних вызовов, придется рефакторить.
Если придерживаться этого правила, код будет более понятным, и придется меньше рефакторить при изменениях требований.
Видел я такой код, где все низкоуровневые действия обёрнуты в функции, зачастую в функции-однострочники:
void ShowWindow(Window window)
{
window.Show();
}
void SetGlobalState(int state)
{
g_State = state;
}Может, это чем-то и оправдано, но мне показалось это лишней когнитивной нагрузкой, возникшей на пустом месте. Мало того, что надо помнить стандартную библиотеку и назначение переменных, теперь ещё и запоминать название и действия всех этих функций-однострочников, чтобы, когда их видишь, лишний раз не листать куда-то вниз, посмотреть, что они на самом деле делают.
А где тут публичные функции, вызывающие ненужные приватные? Это немного не то, о чем я говорю. Я говорю, что например в данном случае другая публичная функция для установки состояния должна вызывать не SetGlobalState(), а g_State = state. Вдруг вы завтра сделаете там проверки нового состояния с выбросом исключения, а в другой функции надо на время ее работы поставить невалидное состояние, которое она в конце работы превращает в валидное.
Это буквальное следование принципу
правильная мысль "Avoid mixing different abstraction layers into a single function"
Не говорю, что это неправильно. Но читать это было… кхм… трудновато.
Не могу сказать, что в этом коде есть буквальное следование этому принципу. window.Show() не является низкоуровневым действием для ShowWindow(), это один уровень абстракции.
На самом деле, если у нас есть функция ShowWindow(), она не должна вызывать window.Show(). Это может сделать и сам вызывающий код. То есть или мы имеем ShowWindow() с внутренними шагами, или window.Show() с теми же внутренним шагами. Это можно сделать по техническим причинам для совместимости (например, для упрощения перехода между мажорными версиями библиотек), но не как самоцель.
От бизнеса или от техлида.
А почему требования какого-то конкретного техлида выдаются за основания для соблюдения универсального правила о невызове публичных функций из публичных? ;)
Я всё подводил-подводил к упоминанию волшебных слов «API» или «библиотека», но, видимо, стоит раскрыть карты и написать прямо.
Я давно обратил внимание, что библиотечный код (включая сюда разные API) и прикладной код — это два совершенно разных вида кода. А ещё я обратил внимание, что когда их путают, получаются разные конфузы.
Самый большой конфуз, на мой взгляд, случился, когда Степанов объяснил, что он старался сделать свою библиотеку максимально дружественной к тем, кто пишет алгоритмы (в сиплюсплюсном понимании этого слова). Но большинство программистов не пишет алгоритмы (в сиплюсплюсном понимании этого слова), а только использует их. А вот к тем, кто использует алгоритмы, его библиотека, ставшая стандартной, повёрнута задом. В отличие от дотнетной, например. Или того же qt.
В данном же случае я думаю, что эту идею можно сформулировать так. Если мы заменим слова «публичная функция» на «библиотечная функция» или «функция API», то я, пожалуй, соглашусь, что есть причины не вызывать их друг из друга. Потому, что там чёткая граница «прикладной код» — «библиотека/API» (и логгирование пересечений этой границы, например) действительно имеет смысл.
А если мы говорим про обычный класс в прикладном коде, то общего смысла в этом нет. Есть только набор частных случаев типа «потому, что техлид так сказал» (довольно плохое объяснение для правил проектирования, не так ли?).
А почему требования какого-то конкретного техлида выдаются за основания для соблюдения универсального правила
Потому что это не требования конкретного техлида, а часто встречающаяся ситуация с требованиями от разных техлидов или представителей бизнеса.
Если мы заменим слова «публичная функция» на «библиотечная функция»
Нет, эта рекомендация работает в рамках любого класса с публичными функциями. Если вы пишете реализацию — пишите реализацию, не вызывайте абстракцию из реализации.
Есть только набор частных случаев
Любой код это частный случай. Из набора частных случаев складывается статистика. Вот как раз с моей точки зрения вы придумали какой-то частный случай, где это необязательно, и пытаетесь с его помощью доказать, что эта рекомендация неправильная для большинства случаев.
Рекомендация правильная, даже если из нее есть исключения. Это ведь не только я говорю, вон даже Google это в блоге написали — "Avoid mixing different abstraction layers". Без кода функций Display(), Show() и Move() нет смысла спорить о деталях. Если дадите ссылку на конкретную реализацию, можем обсудить.
Ок. Если требуется реализовать интерфейс с функциями Move, Show и т.д., мы сразу пишем заготовку
void Show()
{
ShowImpl();
}
void Move()
{
MoveImpl();
}и т.д., и уже спокойно вызываем MoveImpl из ShowImpl.
Но эти однострочные функции нарушают принцип YAGNI.
Вот когда потребуется логирование, тогда и отрефакторим. А заранее забивать себе голову этими Impl… Наверное, можно к этому привыкнуть, если "техлид" так хочет.
мы сразу пишем заготовку
А заранее забивать себе голову этими Impl…
Обычно бывает наоборот, у нас уже есть публичная функция без всяких Impl, мы делаем новую, и хотим вызывать существующую публичную. Если сейчас вынести во внутреннюю функцию, то это будет не заранее, а вовремя.
Функции типа Display(), которые вызывают публичные функции в нужном порядке, часто помещают в отдельный класс Builder/Manager/Utils. А внутри класса лучше работать с деталями реализации. То есть например вызывать this.property = value вместо this.setProperty(value). Публичные функции предполагают, что состояние объекта до и после вызова является валидным, что может быть не так если такой вызов это часть логики другой публичной функции.
Как по мне, YAGNI не годится в фундаментальный принцип. Возьмём для примера универсальность при проектировании. Когда её слишком много, получается архитектурная астронавтика. Но когда по принципу YAGNI об универсальности думают слишком мало — тоже ничего хорошего.
Если разумное количество универсальности было заложено, но она не пригодилась (мы неправильно спрогнозировали сценарии), это лучше, чем когда мы намеренно делаем систему менее универсальной, а потом городим костыли.
В данном случае я бы сказал, что такие однострочники не нужны не по принципу YAGNI, а просто потому, что такая архитектура не решает никакой реальной проблемы. Но про это спор идёт в другой ветке :)
Вот как раз с моей точки зрения вы придумали какой-то частный случай, где это необязательно, и пытаетесь с его помощью доказать, что эта рекомендация неправильная для большинства случаев.
Это называется контрпример. И, вообще говоря, одного контрпримера достаточно, чтобы опровергнуть правило.
Но дело не в этом. Бремя обоснования некоторого правила лежит на том, кто это правило предложил. Я пока такого обоснования не увидел.
Про логгирование я ответил, что как раз задачи такого типа, как логгирование публичных вызовов это частный случай. Ну а про то, что «мне техлид так сказал», так техлиды много чего говорят. Откуда только потом плохой софт берётся.
Без кода функций Display(), Show() и Move() нет смысла спорить о деталях. Если дадите ссылку на конкретную реализацию, можем обсудить.
Нет уж, позвольте. Мы не спорим о деталях. Мы спорим о принципе. А это не детали, а пример, на котором мы этот принцип обсуждаем.
Нужна реализация — пожалуйста. Я сразу написал, что функция Display() — сахарная, чтобы не расписывать в деталях имплементацию. Могу развернуть слово «сахарная» в код, мне не трудно:
void Window::Display()
{
Rectangle rectScreen, rectWindow;
GetScreenRect(rectScreen); // Default screen.
GetWindowRect(rectWindow);
rectWindow.PutInCenterOf(rectScreen);
Move(rectWindow);
Show(ShowWindow::Show);
}Излишняя мутабельность кода обусловлена тем, что писал левой ногой выбранным ЯП.
Реализация Move() и Show() не важна. Но если очень хочется, могу расписать и её.
И, вообще говоря, одного контрпримера достаточно, чтобы опровергнуть правило.
Вообще-то нет. Исключения из правила не отменяют существование правила. Одного контрпримера достаточно, чтобы опровергнуть квантор всеобщности, не каждое правило его имеет. Причем вы даже не опровергли, а сказали "мне неохота писать обертки". Ну неохота, не пишите.
Это правило о том, чтобы иметь меньше проблем с новыми требованиями. Опровержение может заключаться только в том, что при следовании правилу возможных проблем больше, чем без следования.
Бремя обоснования некоторого правила лежит на том, кто это правило предложил. Я пока такого обоснования не увидел.
Обоснование я привел. Из того, что вы не считаете обоснование достаточным, не следует, что я его не привел. Вы можете показать некорректность логических выводов в этом обосновании, но пока я этого не увидел.
Мы спорим о принципе. А это не детали, а пример, на котором мы этот принцип обсуждаем.
Да, только мои объяснения этого принципа вы не понимаете, мои примеры игнорируете, почему-то называете ваш пример показывающим принцип, а мой пример с логированием частным случаем, который не показывает принцип.
Я сразу написал, что функция Display() — сахарная
Я не знаю, что означает термин "сахарная" применительно к функции, поэтому мне он ничего не говорит, даже если вы его сразу написали.
С моей точки зрения функция Display() должна быть не в Window, а в каком-нибудь WindowManager. Там можно сделать не только помещение в центр, а еще кучу разных комбинаций — помещение в левый угол, разворачивание на одну половину экрана, и т.д. Если пихать это всё в WIndow, он превратится в God-object.
Во втором комментарии я написал, что можно сделать так, как вы хотите, если на то есть обоснованные причины, и если вы согласны решать возможные проблемы. Эта рекомендация для тех, кто возможные проблемы решать не хочет. Я не понимаю, что вы мне пытаетесь доказать.
Обязательно городить Window::ShowImpl()/Window::MoveImpl()?
Иногда имеет смысл скопировать в Display() внутренние шаги Show() и Move() со специфичными для Display() параметрами. Тогда добавление новой функциональности в Show() и Move() не повлияет на Display(). Зависит от того, подходит ли нам такое поведение, или нам надо любую новую функциональность повторять в Display().
Это будет нарушением DRY (нормализации), а DRY это очень фундаментальный принцип, который хорошо обоснован. Конкретно: опасность, что в Show()/Move() при вызове «внутренних шагов» появится какое-нибудь важное условие, а мы его забудем добавить в Display(), гораздо больше, чем та, что новая функциональность всё сломает. Тем более, Display() упомянута как «сахарная».
Вот как раз если не хотите нарушать DRY, то надо сделать ShowImpl(). Но на самом деле так редко бывает. Обычно в функциях типа Show() есть больше одного шага, из которых в Display() надо вызвать только главный.
опасность, что в Show()/Move() при вызове «внутренних шагов» появится какое-нибудь важное условие, а мы его забудем добавить в Display()
Неа) Если мы поменяем внутренний шаг, который вызывается и из DIsplay() и из Show(), то поведение поменяется в обоих. Просто очень часто со временем в Show() появляются свои требования, которые не нужны для Display(), отсюда и рекомендация не вызывать одну из другой.
Узнаёте? Это всего лишь код функции слева, к которой в качестве комментариев добавлены имена функций справа.
Комментарии легко теряют актуальность, если при изменении кода про них забыли. Поэтому лучше сводить использование строк в коде к минимуму. Это как при работе с апи надёжнее писать что-то вроде api.order.get(id), чем request(f'/api/odrer/{id}'), потому что если во втором случае будет допущена опечатка, то она может быть замечена не сразу.
Комментарии легко теряют актуальность, если при изменении кода про них забыли.
Иногда вижу это замечание, когда речь захоит о коммитах, и оно всегда кажется мне диким. Да, блин, комментарии — это важная часть разработки, и нужно поддерживать и в актуальном состоянии. Такая же важная, как написание и поддержка документации. Как и составление правильных и полезных комментариев коммитов в гите. Как написание кода, наконец.
Насчёт коммитов вы абсолютно правы, но коммиты не устаревают, в отличие от комментариев. Для меня комментарий - это всегда "запах" и повод задуматься над тем, достаточно ли выразителен сам код. Как правило, я предпочту что-то вроде вериЛонгВарайаблНэйм, потому что комментарий, как вы сами сказали, это дополнительные расходы на поддержку. Не хочу дополнительных расходов там, где можно обойтись без них.
Я бы сказал, что нужность / оправданность комментариев сильно зависит от самих комментариев и как они используются.
Например, комментарии к модулям, классам и заголовкам функций (типа JSDoc / Doxygen / Rustdoc / etc.) часто используются IDE для показа подсказок при автодополнении или для автогенерации онлайн-документации - их определённо стоит поддерживать, хоть это и дополнительная работа.
С другой же стороны, что-то вроде
// convert inches to millimeters
length *= 25.4;действительно лучше было бы выделить в функцию с говорящим названием, тогда и комментарий стал бы не нужен.
Это опечатка, я имел в виду «когда речь заходит о комментариях»)Иногда вижу это замечание, когда речь захоит о коммитахНасчёт коммитов вы абсолютно правы
Для меня комментарий — это всегда «запах» и повод задуматься над тем, достаточно ли выразителен сам код.Каким бы выразительным ни был код, он достаточно хорошо документирует, в основном, поведение, то есть «что в этом месте происходит». Состояние (например, после очередного шага алогритма) из кода вывести гораздо сложнее, потому нужно по сути самому проделать в уме необходимые операции. Наконец, отдельные аспекты невозможно понять из кода иногда абсолютно никак. Например, назначение — «зачем этот код вообще нужен в этом месте, какой цели он служит».
Эти вещи и нужно документировать. А комментарии, повторяющие код, в духе
// Increment the counter.
++counter;Я высказался, наверное, чересчур категорично. Просто мы тут джейсоны гоняем, без всяких алгоритмов, и в 99% случаев можно написать понятно, не используя комментарии. Совершенно согласен с вами. Если смысл кода не очевиден (особенности чужой библиотеки, сложный алгоритм, редкий случай), то без комментария всё становится похожим на магию, что, на мой взгляд, гораздо хуже, чем избыточный комментарий.
Такой вопрос: а как вы относитесь к комментариям типа @todo?
Да, блин, комментарии — это важная часть разработки, и нужно поддерживать и в актуальном состоянии
Согласитесь, было бы проще, если бы компилятор мог автоматически проверять актуальность комментариев и выдавать ошибку, если они устарели.
Тогда бы они перестали быть комментариями, а стали частью кода.
Ага. Поэтому, как вариант - вместо добавления комментария обернуть кусок кода в функцию с понятным названием.
Я не об этом. Чтобы компилятор мог проверить актуальность комментария, эти комментарии должны писаться на формальном языке
Например, вместо
/* объект T должен быть уткой */
надо писать
Obj<T> where T : Duck
В вашем примере, никто не мешает разработчику дописать в функцию пару действий и не обновить её название. Это даже хуже, чем не исправить комментарий. Потому что неактуальный комментарий над действием сразу видно, а чтобы понять, что название функции не соответствует её реализации, надо переходить к ней.
Вы когда-нибудь видели методы по 100500 строк?
я почему-то уверен что автор оригинального текста - не просто видел, а еще и с удовольствием сам пишет такие методы :)
Мой рекорд увиденного - около трёх тысяч строк на питоне; эдакий огромный switch-case от переданного аргумента
Я видел код мобильных игр на уже давно мертву(на телефонах) j2me. Вся довольно не маленькая игра типа Соника была ровно в 1 файле и ровно 2 функциях. Первая очень короткая функция вычитывала все события от пользователя и помешала их в главный цикл. Вторая собственно главный цикл, свитч на ~70 евентов, каждое плечо в среднем ~700 строк кода. И это была типичная структура для игр тех времён на Яве. Ну т.е. метод длинною ~50К строк я видел.
Моя задача была портирование игр, с Ява на с++ и/или с телефона одного размера экрана/вида кнопок на другой, называется пост продакшн.
Видел и поболее, кроме того, ещё и с применением GOTO внутри....
Видел шесть с половиной тысяч строк - switch/case управления банкоматом. Только не метод, а просто такая функция на Си.
Когда приходиться разбираться с логикой кода, отлаживать, вносить изменения, то в правом примере это сделать проще. Сначала выделяется общая логика метода, и нужно ли что-то в ней менять. Потом если с общей логикой все хорошо, то можно смотреть только на один метод и разбираться уже с ним.
Этот пример достаточно простой, поэтому левый код может читаться не хуже правого. Но если у нас будет добавляться какая-то новая логика, появляться больше опциональности, переиспользование шагов, добавится логирование, обработка исключительных ситуаций, потом появится необходимость в тестах, то все равно придется менять запись в сторону правого варианта иначе у нас станет лютое нечитаемое спагетти.
А чего удивляться, да, в гугле не умеют писать код. Это же не новость
Начиная с названия функции - createPizza, вообще-то createPizza это первая строчка с созданием (вызов конструктора), функция на самом деле обрабатывает заказ, а не создаёт пиццу
Далее про "деление на абстракции", чтобы на них делить они... должны быть и к тому же быть осмысленными. Разделение кода как справа на бездумные куски текста плодит множество занимающих место функций с громадным количеством прекондишнов, которые нигде не написаны. Ничего не остаётся, кроме как никогда не использовать такую функцию нигде(либо прочитать что там внутри, понять какие прекондишны у функции и написать их комментарием)
В комментариях пишут, что код слева/справа легче читается, но как будто никто не прочитал код полностью И слева, и справа написано по 28 строк, но слева на "обёртки" (заголовок + фигурные скобки - границы блока) функции ушло ровно 2 строки, а справа - 11 (немногим меньше половины, Карл!), причём по меньшей мере 3 функции там объявлены, но не реализованы.
Я понимаю, почему некоторые считают правый вариант более удобным/понятным, но есть же граница, когда дробление кода приносит больше синтаксического мусора, чем пользы. Тем более, очень это странное дробление:bake() создаёт oven с параметрами по умолчанию, но настройка = разогрев вынесен в отдельную функцию? Вы уж или factory / builder используйте, или настраивайте в этом же месте, а то ни туда, ни сюда. И почемуpizza инициализируется нужными значениями Base, Order и Cheese сразу, а не в 3 отдельных функциях?
func makePizza(order *Order) *Pizza {
return pack(bake(prepare(order)))
}
Надеюсь, что мозг сломал всем.
А если без шуток, то вариант слева прекрасен своей лаконичностью, но практически нерасширяем. Так джуны пишут в стартапах. А еще так пишут сеньоры код на выброс.
Вариант справа расширяем, для него можно писать тесты и т.д. Можно сменить тип духовки или добавить двухуровневый разгрев и т.д. Так обычно пишут опытные разработчики успевшие хлебнуть всякого и им просто хочется написать нормальный код и пойти домой. А когда надо что-то поправить, не пытаться вкурить 20 строк превратившиеся в 2000, а просто исправить одну функцию не нарушая интерфейсов.
А по большому счету оба фрагмента воняют индусятиной за версту. Странный выбор наименований и плохая семантика в целом.
Если у пиццерии летом поставят столики и понадобится отдавать пиццу без упаковки, то левый код придётся копипастить почти весь, а в правом только добавить один метод из трех строк createPizzaWithoutBox()
Обычно это решается введением параметра "без коробки".
Тут тоже холиварный вопрос: что хуже — функция-монстр с 10 параметрами, или 250 простых функций, которые делают ровно то, что написано у них в заголовке. Второе проще читать, но если потребуется внести изменение в одном параметре, которое затронет сразу 60 вариантов...
Оххх. Классный пример попинать ООП. :) Типичные проблемы oven.Insert(pizza) или pizza.ПолезайВ(oven)
И забавные абстракции pizza.Boxed = box.PutIn(pizza) -- т.е. внутри пиццы теперь у нас будет лежать еще и упакованная пицца. :) А потом еще и .Boxed , и Sliced и .Ready... оййоой....
; условный лиспоподобный псевдокод без синтаксиса отступов
; для базовой ситуации, где одна печь
defn produce-pizza [order]
; включаем нагрев печки
oven/start-heating our-pizza-cooking-temperature ; берем нашу стандартную Т
; "создаем" новую пиццу
let [pizza (create-new-pizza order "Mozarella")] ; я указал как в примере
; ожидаем полного нагрева
oven/wait-for-temperature our-pizza-cooking-temperature
; .......
; а дальше конвеер операций c pizza
(-> pizza
(add-toping (get order :kind))
(oven/bake our-pizza-cooking-time)
put-in-a-box
(slice (get order :size))
close-the-box)
; Нам же важен порядок манипуляций?
; Значит вполне логично выразить это в последовательности операций
; Каждая следущая работает с результатом предыдущей
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
; ну и локальные функции для работы с pizza:
defn add-toping [pizza kind] ...
defn slice [pizza size] ...
defn put-in-a-box [pizza] ...
defn close-the-box [pizza] ...А если у нас печей много, то должно быть что-то вроде такого, т.е. операции в контексте печи с этим ID:
def cooking-params {:temp 221, :time 20}
......
defn produce-pizza [order]
with-open [current-oven (oven/find-free-oven)]
; передаем команду "подготовить и начать нагрев до температуры X"
(oven/prepare current-oven (get cooking-params :temp))
; внутри with-open для того, чтобы уже пошел нагрев, пока мы начинаем делать пиццу :)
let [pizza-template (create-new-pizza order "Mozarella")]
(oven/wait-for-temperature (get cooking-params :temp))
(-> pizza
(add-toping (get order :kind))
(oven/bake current-oven (get cooking-params :time))
put-in-a-box
(slice (get order :number-of-slices))
close-the-box)
В этом, на мой взгляд, такой код не сломает мозги непонятными метаниями между лишними абстракциями.
Или вот даже в такую сторону можно еще продвинуться:
(def cooking-params {:temp 221, :time 20})
...
...
...
...
...
(defn produce-pizza [order]
...
with-open [current-oven (oven/find-and-program-oven cooking-params)]
(-> order
(create-new-pizza "Mozarella")
(add-toping (get order :kind))
(oven/bake current-oven)
(put-in-a-box)
(slice (get order :number-of-slices))
(close-the-box))
На мой взгляд ни один вариант не подходит. Если речь про ФП, то скорее нужно что-то в духе pipe(prepare, bake, box)(pizza), если про ООП, то либо реально разносить по «энтерпрайзному» с блекджеком и фабриками, если логики реально много, либо просто применить паттерн builder, что будет достаточно читаемо.
Насчет проваливается в функции для понимания деталей реализации, например если реально важно, будет ли bake второй раз печь уже испеченную пиццу, то можно либо сделать метод с каким-то очевидным названием, который будет сбрасывать в 0 требуемое время выпечки — это даст нужную информацию, или же использовать более длинные названия методов, типа bakeIfIsNotBaked().
Всем, кто пишет комметарии в духе "но левый подход не работает с функциями из тысячи строк" — а что, если я вам скажу, что можно функции на тысячу строк декомпозировать, а относительно небольшие оставлять, как есть?
Люди очень любят придумывать или находить эвристические правила, вроде тех же 7-12 строк, а потом пытаться применять их везде, не задумываясь о том, подходят они к данному случаю или нет. Не надо так.
Люди очень любят придумывать или находить эвристические правила, вроде тех же 7-12 строк, а потом пытаться применять их везде, не задумываясь о том, подходят они к данному случаю или нет. Не надо так.
а как вы думаете - в чем цель некоторых статей?
чтобы в чем-то разобраться (есть ли преимущества у А или Б - и прийти к выводу что у каждого есть свои слабые и сильные стороны) или вызвать поток комментариев от "я за А" против "я за Б", чтобы они спорили между собой - у кого вера крепче?
Вот только если поменять порядок функций в правом варианте и чуть-чуть изменить использование переменных, код станет не сильно отличаться от левого варианта.
Тяжело скакать по функциям, когда они написаны в рандомном порядке, а расположите их одинаково, поставьте все управляющие конструкции внизу -- и проблем не будет.
(Спагетти код -- это плохо, но не потому, что кусочки слишком маленькие -- это вывод, к которому пришёл автор -- а потому что они перемешаны!)
А, соответственно, в левом коде есть обратная проблема: как только куски кода станут больше, ваша монолитная функция превратится в кашу, и, чтобы перескочить с одной части логики на другую, вам придётся читать весь код функции!
Код слева мог бы быть ОК, но если вчитаться, то ничего хорошего в нем нет, в итоге код справа только усугубляет проблемы левой части. Нет обработки ситуации когда kind пришел неизвестный (да и вообще нет обработки ошибок), странные интерфейсы у печи, коробки и тд, вместо oven.Bake(anything, time) написан код, который будет дублирован везде, непонятно зачем нужен флаг Baked, box.SlicePizza - это прям что-то странное. Это все не имеет даже отношения к конкретному языку.
Проблема этого кода в обоих случаях в том, что автор пытается манипулировать объектом, используя для этого функциональное программирования, вынуждая каждый раз передавать контекст. Если добавить щепотку ООП, то станет проще.
class Oven:
def heat()
def bake(product, kind)
class Pizza:
def __init__(self, size, kind, sause, cheese)
# присваиваем переменные в self, запускаем методы
self.add_toppings_by_kind(kind)
def add_toppings_by_kind(kind)
# делаем свитч
def bake()
oven = new Oven()
oven.heat()
oven.bake(self, "pizza")
def wrap_up()
# упаковываем
pizza = new Pizza("размер", "вид", "соус", "сыр")
pizza.bake()
pizza.wrap_up()
Линейный код более читаем