Комментарии 69
с каждой новой редакторской версией автор сам исправляет себя
Автор, видимо, пользуясь своей же методикой, постепенно делает рефакторинг книги :) «Чистый текст» про «чистый код».
Естетственно, будут возникать противоречия при таком подходе. Конечно, книга не очень подходит новичкам, которые только начинают изучать программирование. Но, на мой взгляд, это не потому что книга противоречивая, а потому что новички еще не знакомы с чужим кодом и они, по сути, ещё не понимают о чём эта книга.
Плюсую от части. Скорее это одна из самых слабых книг. В целом там все верно описано, но оно годится только для начинающих. Мало мальски опытные люди, которые читают чужой код, рано или поздно, сами приходят ко всему тому, что там описано
Мне в этом смысле нравится не столько «что делать», но описание «почему мы пришли к выводу, что так нужно делать, какую проблему это решало» — самые ценные книги как раз содержат примеры кейсов, и если их анализировать, можно понять, какие вещи и почему тебе не подходят или нужно адаптировать.
Если универсальных рецептов нет — неуниверсальные очень неплохи
Главное без фанатизма.
Использование экземпляров классов вместо функций с параметрами — та же история. По функции с параметрами гараздо проще понять что она должна сделать, нежели метод, с объявленными где-то там атрибутами.
Книгу не читал и не буду настаивать на ее необъективности, но чаще всего это книги для общего развития.
вот пример из реального проекта от профессионалов:
def reset_label(cls):
"""
reset intenal counter
"""
cls.last_label = 0
Метод не переиспользуется. В чём смысл из одной строки делать две, да ещё и описывать новый метод для этого?
Где надо остановиться? Надо ли из кода:
print("Hello, world!")
делать нечто такое:
def printHelloWorld():
"""
напечатать приветствие
"""
print("Hello, world!")
чем оно лучше?
В первом примере усматривается, что название функции, комментарий и тело функции не синхронизированы в деталях об именовании счётчика.
Во втором, например, можно было бы убрать комментарий и назвать функцию printHelloMessage.
Оба предложения — лишь догадки о том, какую функцию этот код должен выполнять.
В первом примере усматривается, что название функции, комментарий и тело функции не синхронизированы в деталях об именовании счётчика.
Согласен, есть какое-то несоответствие в наименованиях. Но надо ли было вводить этот метод?
Обязательно нужен контекст, где этот код написан и каково его назначение.На счёт контекста не совсем понял, какой именно части Вам не хватает. Можете привести примеры, где подобный подход действительно себя оправдывает? С переиспользованием кода мне всё понятно, но тут…
private static StreamingOutput getListingStream(final NamenodeProtocols np,
final String p) throws IOException {
// allows exceptions like FNF or ACE to prevent http response of 200 for
// a failure since we can't (currently) return error responses in the
// middle of a streaming operation
final DirectoryListing firstDirList = getDirectoryListing(np, p,
HdfsFileStatus.EMPTY_NAME);
// must save ugi because the streaming object will be executed outside
// the remote user's ugi
final UserGroupInformation ugi = UserGroupInformation.getCurrentUser();
return new StreamingOutput() {
@Override
public void write(final OutputStream outstream) throws IOException {
final PrintWriter out = new PrintWriter(new OutputStreamWriter(
outstream, Charsets.UTF_8));
out.println("{\"" + FileStatus.class.getSimpleName() + "es\":{\""
+ FileStatus.class.getSimpleName() + "\":[");
try {
// restore remote user's ugi
ugi.doAs(new PrivilegedExceptionAction<Void>() {
@Override
public Void run() throws IOException {
long n = 0;
for (DirectoryListing dirList = firstDirList; ;
dirList = getDirectoryListing(np, p, dirList.getLastName())
) {
// send each segment of the directory listing
for (HdfsFileStatus s : dirList.getPartialListing()) {
if (n++ > 0) {
out.println(',');
}
out.print(JsonUtil.toJsonString(s, false));
}
// stop if last segment
if (!dirList.hasMore()) {
break;
}
}
return null;
}
});
} catch (InterruptedException e) {
throw new IOException(e);
}
out.println();
out.println("]}}");
out.flush();
}
};
}
Вот как примерно выглядит результат работы
{"FileStatuses":{"FileStatus":[
{"accessTime":0,"blockSize":0,"childrenNum":1.....},
{"accessTime":0,"blockSize":0,"childrenNum":1......},
{"accessTime":0,"blockSize":0,"childrenNum":37.....}
]}}
- Вы (как человек работающий с этим кодом) понимаете смысл действия
cls.last_label = 0
?
Я — нет, но благодаря этой функции я смогу сбросить label.
Итого:
- функция выглядит полезной
- комментарий не соотносится с названием функции.
- Функция
printHelloWorld()
нужна в том случае, если определенный набор действий (print("Hello, world!")
) повторяетсябольше 2х размногократно.
P.S. Все вышесказанное = имхо.
cls.last_label = 0 # reset intenal counter
Я тут вижу большой плюс в том, что мне не придется искать описание этой функции/метода где-то ещё и сразу могу понять что именно делает эта строка.
Функция printHelloWorld() нужна в том случае, если определенный набор действий (print(«Hello, world!»)) повторяетсябольше 2х размногократно.
Предположим, что так, хотя метод из профессионального проекта вызывается только единожды. Но тут возникает вопрос о том, насколько это уместно. Чем оно лучше print(«Hello, world!»)? Оно так же очень хорошо читается.
При многократном использовании я вижу только один плюс: не сделать ошибку в словах. Но читаемость при этом ухудшается (IMHO). А что на счёт производительности?
Можно только гадать почему не использовался такой вариант:
cls.last_label = 0 # reset intenal counter
Может из-за того, что в будущем (для сброса) потребуется больше 1 действия. Ответить сможет только автор кода, если вспомнит.
По поводу printHelloWorld()
: вы сами ответили на свой вопрос.
Как минимум снизится шанс опечатки. За счет того, что действия находятся в 1 месте.
Отсюда следует другой плюс: чтобы изменить эти действия придется внести правку в 1 место, а не искать все места с копипастой по всей кодовой базе.
Насчет быстродействия: на абстрактном примере нельзя дать никакой другой рекомендации кроме "код надо писать для человека" (ибо может оказаться что узкое место вообще в другом месте и т.п.).
И самое главное (снова имхо): на подобные вопросы нет однозначного ответа. Все зависит от контекста.
Может из-за того, что в будущем (для сброса) потребуется больше 1 действия.Не думал про такой вариант, так как ничего не мешает в будущем этот метод описать. То есть автор заранее закладывает функционал, который, возможно, никогда не будет использован? Это считается правильным подходом в профессиональной среде?
Отсюда следует другой плюс: чтобы изменить эти действия придется внести правку в 1 место, а не искать все места с копипастой по всей кодовой базе.Неочевидной плюс (IMHO). При изменении наполнения функции (print(«Hello, world!») -> print(«Welcome!»)) придется изменить её название (для соответствия) и тогда точно также копипастой менять по всей кодовой базе. Если же придумать универсальное название, то не будет очевидно что именно она делает.
если не выделять эту строчку в функцию, каждый раз придется создавать лямбду и т.д.
То есть автор заранее закладывает функционал, который, возможно, никогда не будет использован?
Возможно наоборот — там был какой-то дополнительный функционал (или переиспользование) в предыдущей или исходной версии кода. В данной итерации кода функционал оказался не востребован. Можно было бы зачистить. Но если это не критично, можно и так оставить.
Я тут вижу большой плюс в том, что мне не придется искать описание этой функции/метода где-то ещё и сразу могу понять что именно делает эта строка.Если вам нужно искать описание ЧТО делает эта функция, то есть проблема с именованием функции, а для чтения кода для понимания того «ЧТО тут происходить», не должно интересовать то КАК функция выполняет свою задачу. Это просто разные уровни абстракций опять же.
Тут польза в том, что от пользователя функции скрывается конкретный механизм реализации. Сегодня мы присваиваем 0 какому-то свойству, завтра None, потом у нас будет сложный объект, а то и иерархия с метками на нескольких языках, послезавтра целый файл labels удалим или в базе таблицу. А для пользователя останется всё та же reset_label. Ему без разницы, что происходит внутри, он знает только как сбросить метку — вызвать reset_label
Смысл в выделении такой функции — выделение подписанной абстракции, которая лучше для понимания.
Оправданно это в нескольких случаях:
— для разделения ответственностей
— для оборачивания сложной логики
Несмотря на то, что сам Мартин рекомендует оборачивать даже одну команду в функцию, обычно это бред. Просто так выносить стоит только те абстракции, которые хорошо выделяются
def onWindowShown(self):
// show canvas
self.canvas.showRect(widht, height);
self.canvas.setColor(back_color);
// set controls state
self.start_button.setActive(player.isReady())
self.exit_button.setActive(!player.IsTutorialStarted())
// spawn effect
self.stars_effector.SpawnStars(self.start_button.position)
Представьте, что этого кода в 2 раза больше, а с развитием проекта этот код легко может вырасти в 4 раза (больше эффектов, сложнее логика)
Он станет значительно лучше, если абстрагировать операции
def onWindowShown(self):
self.showBackground(widht, height, back_color);
self.setControlsState(player.isReady(), !player.IsTutorialStarted())
self.showStartEffects()
Второй момент — сложные сравнения, тут даже пример с одной строчкой можно
// вместо
if (user.account.balance > 0 and user.payment_mode != 0 and !user.account.isBanned):
doIt()
// сделать
if (user.ReadyToPay()):
doIt()
Тут имеется смысл, потому что неявный набор сравнений теперь поименован
Третий вариант (как в вашем примере) — абстрагирование для других абстракций
Ваш пример
def reset_label(cls):
"""
reset intenal counter
"""
cls.last_label = 0
Выглядит глупым (возможно так и есть), но представьте, что у вас есть экран, и потребители используют у него только методы
set_label(cls)
и
reset_label(cls)
set_label оправдан — он сложный, и устанавливает у cls множество разных флажков и примочек, помимо текстового значения.
в таком случае reset_label правильно сделать именно так — иначе получится, что включаешь ты через метод, а выключаешь — через ж*пу
Еще вариант — метод reset содержал еще инструкции и был оправдан, но они умерли с изменением проекта
Но скорее всего просто абстракция ради абстракции случайно получилась)
в таком случае reset_label правильно сделать именно так — иначе получится, что включаешь ты через метод, а выключаешь — через ж*пуо! Это оно! Спасибо за объяснение и с именованием сложного сравнения тоже отличный пример.
1. К примеру, у тебя есть класс А, который использует класс B
Все отлично
2. Приходит новая задача — класс А должен уметь иногда вместо класса B использовать класс С
Ты пишешь if по параметру, вставляешь класс C — задача решена
Все отлично (правда отлично, если бы история закончилась тут — ты был бы молодцом, твой код был бы хорошим, тобой были бы довольны начальник, жена, мама и бабушка)
3. Приходит новая задача — класс А, иногда должен уметь вместо B и C использовать класс D. И в этом случае еще по другому надо обрабатывать часть класса А.
И тут у тебя выбор:
3.1 — Написать еще пару if и воткнуть класс D в пару мест — быстро и просто (хотя может уже не столь идеально)
3.2 — Понять, что класс D, как B и C имеет общие детали работы, и их можно обобщить, частично унифицировать, а часть функциональности вынести в Z. Будет гораздо лучше лучше, чем п3.1, и лишь чуть сложнее, чем в п.1 начале
Но потребует переписывать и класс A, и класс B и класс C, а они уже написаны и протестированы, на это потребуется больше времени, чем п3.1. Начальство негодует.
Уже тут у тебя есть выбор, каким молодцом быть. В одних случаях недоволен будет начальник, в других — бабушка (хороший же мальчик!).
4 А ведь проект развивается!
4.1 Естественно, приходят еще добавления новых режимов обработки E(/F/G/ ?)…
И если ты выбрал 3.2 — то тебе нормально, а вот если 3.1 — то опять нужно решать, либо ты плодишь if-ы, делая код менее читаемым, либо менять нужно уже ABCD/
4.2 Естественно, новые режимы E(/F/G/ ?) не нужны и не приходят к тебе никогда. Тогда, выбрав 3.1 ты на самом деле молодец, а в следующем обновлении вы выкинете класс А целиком и напишите другой, более красивый.
И в общем заранее не угадаешь.
1.1, ретроспективный пункт
Ты — умелый и опытный программист, который осознает, что для задачи A + B, наверняка потребуются C/D/E
Сроки не давят, можно попробовать предусмотреть разумные точки расширения (YAGNI будет не прав, если тебе это понадобится). Поэтому ты делаешь 3.2 сразу. Да, немножко оверинженеринг, и времени для красоты заняло больше. Непонятно, окупится ли. Но если появятся C/D/E — будет меньше боли.
1.1.1 — приходит коллега и не понимает, зачем тут метод с одним аргументом.
Мне больше понравился ваш пример по унификации обращений к функциям. vdshat ниже тоже пишет о таблицах методов.
Любое закладывание на будущее это как ставка на тотализаторе :) Иногда почти 100% угадаешь, иногда шансов нет. Ну и начальник, если не ясновидящий, ничем не поможет в вопросе "какая у нас нагрузка будет?" в случае условного стартапа. Может рассказать о планах типа "все пользователи фейсбука к нам перейдут и будут в 2 раза больше впремени проводить, и в 3 раза чаще клацать"… Но то такое...
def set_readonly(list):
list.is_enabled = 0
А назавтра заметил хитрый баг и поменял функцию:
def set_readonly(list):
list.is_enabled = 0
change_button.is_enabled = 0
Смешно писать на языке, которого я не знаю, но идея думаю ясна. Вынеся одну строчку в отдельную функцию, я меняю эту функцию, использующуюся в нескольких местах, в одном месте, при этом код полностью объясняет сам себя. Даже если изменения бы не потребовалось, так код было проще читать чем раскиданное по коду свойство с неочевидным названием (из чужой библиотеки).
def set_readonly(list):
list.is_enabled = 0
change_button.is_enabled = 0
Не совсем удачный пример, так как, мне кажется, что вы не следуете принципам «чистого кода» здесь. Разве не следовало написать ещё один метод (def disable_button)? А потом написать ещё один метод, который вызывал бы эти 2 за один раз. Ну и заодно поменять то место, откуда происходил вызов set_readonly.
Метод, название которого равно содержимому, на мой взгляд уже мало полезен:
def disable_button(button): button.is_enabled = 0
использование его содержимого уже не ухудшает чтение кода, может даже наоборот
cls.last_label = ""
а кто-то так
cls.last_label = None
void openFile(){
// Открываем файл
close(f);
}
Я тогда поудивлялся немного, но списал это на корпоративные требования: там действительно, помимо шапки комментариев для каждой подпрограммы, комментарием была снабжена и каждая строка кода.
А еще я встречал метод Read который внутри вызывал метод Write на себе с константами вместо недостающих параметров. По сигнатурам понятно что оно делает и почему так написано, но именование неадекватное, так что волосы дыбом.
В реальности могут быть ситуации когда оба стиля оправданы.
Книга которая после прочтения второй раз 10 лет спустя очень сильно меня изменила. Я стал действительно понимать что имеется ввиду и сразу из опыта возникали примеры почему это плохо. Примерно как читать Мастера и Маргариту на уроке литературы в 13 лет и потом прочесть в 25.
И тем не менее я всем ее всегда рекомендую. Не смотря на то что чистый код как и любое искусство открыто для интерпретаций.
Далее из вики: Гала́ктика Сомбре́ро — спиральная галактика в созвездии Девы на расстоянии 29,3 млн световых лет от Земли. Диаметр — около 50 000 световых лет — примерно 30 % диаметра Млечного пути
А как следствие, пропорционально возрастающее количество связей между ними вас не смущает?
«Чистый код» как чисто хайп будет актуален еще долго, но если вам ехать лучше что-нибудь из grasp принципов почитать.
Например, есть такая книга «Введение в реальный ITSM» — для тех, кто не в теме корпоративного стиля системного администрирования и не измучен ITIL, там ничего особого нет, но вот лично я увидел там много знакомого, заставляющего взглянуть на него со стороны отнюдь не ITIL.
Функции должны иметь как можно меньше аргументов, желательно ни одного.
Clean code, очевидно не подразумевает написание pure code.
В чистом коде функция не может не иметь ни одного аргумента, ведь ее можно(и нужно) заменить константой.
В остальном — я давно уже пришел к маленьким функциям и маленьким файлам еще.
Ах да, попытка «в лоб» уменьшить количество аргументов обычно приводит к тому, что аргументы схлопывают либо в кортежи либо в plain объекты, что меняет функцию только визуально, проблема не решается. Размер функции и количество аргументов это симптомы болезни, root cause же этой болезни — избыточное количество отвественности в функции. Надо уменьшать ответственность, растаскивая ее на части, декомпозируя то бишь. Количество аргументов и размер функции подтянутся сами.
- Функции должны быть короткими — не длиннее 20 строк и в большинстве случаев менее 10 строк.
- Функции должны иметь как можно меньше аргументов, желательно ни одного.
Хорошо быть теоретиком! «Пишите короткие функции, мойте руки перед едой, пристёгивайтесь за рулём, не употребляйте наркотики».
А вот, например, реальный мир: функция копирования текстуры в движке Хромиума: 14 входных параметров, 150 строк кода. И она, блин, такая, потому, что это уже упрощение после 15 (!) слоёв абстракции, разделения на платформы, упрощения и т.д. И разбить её на «10 строк, в идеале без параметров» можно только введением ещё 14-ти слоёв абстракции, что убъёт производительность и сведёт до нуля количество людей в мире, вообще осознающих, что тут происходит. А вот в этом ужасном виде с 14 параметрами и 150 строками кода — данная функция вызывается на миллиарде девайсов ежедневно, десятки раз в секунду. И работает.
Реальный мир сложен. Сложнее воображаемой волшебной страны с единорогами дядюшки Боба, в которой можно с пафосом рефакторить 20-строковые функции до 10-строковых в проекте на 100 строк в сумме.
При этом не проводится разбор обратных сценариев и нюансов: а когда, собственно, имеет смысл делать большие функции и чем мы жертвуем (опуская производительность), разбивая и вынося подсчёт высоты в отдельный метод. Каким образом и почему в продакшне таки появляются огромные функции? Неужели, все, кто их пишут — некомпетентны и не читали Clean Code?
Пункт про размер функции непонятно как работает со вложенными функциями, но я подозреваю, что в основном языке автора их на тот момент не было.
Пункт про количество аргументов — абсурд, ведь любой набор из Х аргументов эквивалентен объекту с Х полями. Так что, станет лучше, если мы везде вынесем DTO?
Если читать книгу Мартина новичком и воспринимать буквально, на мой взгляд, хорошего не выйдет. А если вы опытный разработчик и понимаете, что картина не всегда такая простая и однозначная, зачем вам эта книга?
Искренне не понимаю, откуда на медиуме набралось 3к лайков и здесь 40 плюсов. После прочтения статьи появилось чувство не детального и обстоятельного разбора, а чего-то маркетингового с кучей обтекаемых и абстрактных формулировок.
Я могу сказать про себя, что начинал с больших функций, как и все. Но чем больше практикуешься в декомпозиции, тем лучше она получается. И код сейчас делится на мелкие функции легко и естественно. Это не бинарный показатель, код не делится на плохой и хороший, это длинная шкала, на которой укладывается многолетний опыт. Хотя иногда конечно получается, что какое то конкретное место пишется хуже, чем в среднем и этот код сам для себя считаешь плохим и думаешь «потом перепишу».
Так же и принцип единственной ответственности, он звучит как бинарное правило, либо ответственность одна, либо две и больше. Но это тоже так не работает, и в этом ошибка первоисточников. А вот если взглянуть на это как на непрерывную шкалу, то оно превращается в «слишком много отвественности» и «достаточно мало». И опять, опыт и практика позволяют со временем смещаться по этой шкале в нужную сторону.
Если же воспринимать эти книги вот так вот, отрицая их, потому что с первого раза не получилось, то с мертвой точки оно не сдвинется естественно.
Других причин писать большие функции нет.
Если у нас есть дублирование некоего кусочка кода в двух функциях, то его вот обязательно выносить в отдельную? Всегда?
Вопрос номер два — если у нас вообще нет дублирования, зачем захламлять класс кучей приватных функций, которые используются в нашей одной большой?
А вот если взглянуть на это как на непрерывную шкалу, то оно превращается в «слишком много отвественности» и «достаточно мало».
Абсолютно согласен.
Если же воспринимать эти книги вот так вот, отрицая их, потому что с первого раза не получилось, то с мертвой точки оно не сдвинется естественно.
Я воспринимаю не «эти книги», я воспринимаю эту конкретную книгу и этого конкретного автора. Есть куда более полезные и сдержанные в своей подаче материалы, а в тысячный раз обсасывать «чистый код» мне лично не хочется. Не исключаю, что я в меньшинстве.
Если у нас есть дублирование некоего кусочка кода в двух функциях, то его вот обязательно выносить в отдельную? Всегда?
Когда придумаешь хорошую абстракцию, которая хорошо подходит к обоим функциям. )
Вопрос номер два — если у нас вообще нет дублирования, зачем захламлять класс кучей приватных функций, которые используются в нашей одной большой?
Разделять уровни абстракции, например. Вот приходит вам ТЗ в виде сценария из 20 абзацев у каждого из которого есть заголовок краткий и описание алгоритма в "теле" абзаца строк на 10. В коде это легко может вылиться в функцию на 1000+ строк. А можно сделать одну функцию из 20 строк и 20 функций по 50+ строк. Это лишнее по вашему?
Когда придумаешь хорошую абстракцию, которая хорошо подходит к обоим функциям. )
То есть не всегда или, как минимум, не сразу.
Это лишнее по вашему?
Зависит от контекста и от наполнения этих самых строк. Собственно, в этом и была моя изначальная мысль — всё не так просто. При этом статья не приводит примеров, где надо делить, где не надо, а лишь говорит «маленькие функции — хорошо». Отличный посыл новичкам, которые ринутся выносить приват методы на каждый чих, ведь так Мартин сказал.
А можно сделать одну функцию из 20 строк и 20 функций по 50+ строк.
Тут, кстати, занятный вопрос — а функция, у которой внутри 50 функций на 20 строк, это всё ещё функция на 1000 строк?
Прочитал совет не связываться с мультипоточностью так как эТа сЛоЖнА.
Поглядел на книжку Goetz на полке. Ту, где мультипоточность предподается правильно и по крайней мере понятно, как делать ее корректно, чтоб чего не вышло.
Закрыл книжку дяди Боба раз и навсегда.
Вообще любые «сухие концентраты для щенков» (с) мой лектор по механике на первом курсе — это не повод вырубать мозг. Когда программист решает задачу, меньше всего ему нужен нудящий коллега, который будет рассказывать о том, что ему не только нужно решить задачу, но дополнительно еще решить ВТОРУЮ задачу УДОВЛЕТВОРИТЬ ДЯДЮ БОБА. Безусловно, код должен быть легко читаемым, модифицируемым, отвечающим SOLID, разбитым на функции. Но вот эти абстракции — а кто нить задумывался, как они тормозят? Думаю, если бы дядя Боб работал на форекс, он просто был бы куда умнее в перформансе и в мультипоточности и делал бы на это поправку.
Принципы из книги не подходят к жизни? Ответ:

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