Comments 19
Однако спасибо за перевод, а автору за статью. Я джун, с реальным опытом всего 2 месяца. И, черт возьми, первый пример — это про меня. Но я одумался и буквально в начале недели отрефакторил свой код правильно. Но этот текст помог объяснить самому себе, почему мне первый код не нравился.
Не используйте булевы значения в качестве параметров
При разработке функции может возникнуть соблазн добавить флаг. Не делайте этого.
Поясню на примере: допустим, у вас есть система передачи сообщений, и есть функция getUserMessages, возвращающая пользователю все сообщения. Но есть ситуация, в которой вам нужно возвращать либо краткую суть каждого сообщения (допустим, первый абзац), либо сообщение целиком. Поэтому вы добавляете параметр в виде флага или булева значения, который называете retrieveFullMessage.
Повторюсь, не делайте этого.
Потому что те, кто будут читать ваш код, увидят getUserMessage(userId, true) и будут недоумевать, что это вообще такое?
ИМХО:
здесь ИМХО нужен особый подход.
getUserMessage(userId, true) очевидно плохо.
Нужны константы с осмысленными именами:
const retrieveFullMessageFl = true; retrieveShortMessageFl = false;
И вызов записывать только с этими константами:
getUserMessage(userId, retrieveFullMessageFl); getUserMessage(userId, retrieveShortMessageFl);
ИМХО писать 2 функции вместо одной не всегда удобно. Когда, нпр., выбор между коротким или длинным сообщениями отладка и модификация двух функций м.б. заметно сложнее, чем одной.
Вы не контролируете все вызовы вашей функции. Другой разработчик может написать вызов с литеральным значением, мотивируя, например, тем, что оно используется только в одном месте.
В добавок, написание таких дополнительных констант полностью нивелирует выигрыш от неразделения функций: приходится писать кода больше линейно по количеству вызовов.
Кроме того, когда появится требование иногда возвращать только аннотацию краткой сути сообшения (допустим, первые 72 символа), этот код всё равно рефакторить. И в каком-нибудь динамически слабо неявно типизируемом языке с редакторами кода не так здорово, чтобы за три секунды это решить.
Так что, речи об отдельной функции для коротких сообщений не идёт.
Однако если ее декомпозировать, то эти две функции начнут выглядеть примерно так.
getShortMessage(id) {
const data = loadMessageData(id);
return composeShortMessage(data);
}
Нетрудно представить как будет выглядеть второй метод при этом.
ЗЫ и да, в той упомянутой статье есть такой пункт — не надо бояться выкинуть свой код в помойку. Конечно, у нас всегда не хватает времени, жмут оценки, давит начальство, «все было против нашей сборной по футболу»(с)
Мне трудно это все доносить, люди, которые пишут код примерно так как здесь,
Я же предполагаю, что код надо писать примерно так и хардкодные параметры, ака флаги, из такого кода испаряются сами собой.
Это примеры кода из реального проекта. их не предполагается внимательно читать, просто взгядом оценить, чтобы понять о чем идет речь.
Второй пример кода лучше не только потому, что он больше декомпозирован, но и еще потому, что получает список сущностей про списку ключей, а не как оригинальный, который получает всегда по одному ключу, что есть проблема N+1. Речь не о ней, конечно же, но стоило упомянуть, что код который лучше не обязан работать медленнее, он может и быстрее работать, чем «то что было раньше».
PS не получилось картинки из г-драйва подключить, работают если в отдельном окне открыть по правой кнопке
То есть вы предлагаете вместо
element.classList.toggle( 'visible' , isVisible )
Везде писать
if( isVisible ) element.classList.add( 'visible' )
else element.classList.remove( 'visible' )
Я правильно понял вашу мысль?
element.classList[ isVisible ? 'add' : 'remove' ]( 'visible' )
</joke>Если я правильно понял посыл автора, то система должна содержать две специализированные функции (add и remove) и, опционально, собранную с параметром (toggle). В первом и особенно во втором примерах обе ветки if-а заинлайнены, а не выделены отдельными именованными функциями.
Избыточная сложность