Конференции Олега Бунина (Онтико) corporate blog
Website development
JavaScript
Programming
Perfect code
Comments 40
0
Доклад может и один из лучших, но по прочтении невооруженным взглядом видятся взаимоисключающие параграфы. Как собственно и всегда, когда речь заходит за сферическую читаемость кода в вакууме.
«Не вкладывайте тернарные операторы»-«не создавайте лишних переменных»-«пишите линейно» — в итоге тут таки будут либо трусы, либо крестик. Вложенные тернарные операторы как раз таки и пишутся, чтоб избавиться от двух последующих пунктов, уберем тернарники — вернёмся к if then else и временным переменным.

И так далее.
-1

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


И если речь идёт о выводе из вакуума: Вы готовы предложить примеры, которые (на Ваш взгляд) ну никак нельзя сделать удобочитаемыми, разумно употребляя предложенные правила?

+3
разумно употребляя предложенные правила?

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

Кажется, без конкретных примеров это всего лишь брюзжание.

Вам серьезно нужны «конкретные примеры кода», чтоб понять, что заменяет тернарник? Да вот это он заменяет:
let a;
if (condition) {
  a = 1;
} else {
  a = 2;
}

А два вложенных — соответственно, иф с ифами внутри, и так далее. Как такой кусок не переписывай, а читаемость у него будет приблизительно одинаково плохая — ну, разве что исключая вырожденный случай типа приведенного в статье, где скобочки блоков будут куда более заметны (и для IDE в том числе), чем где-то заныканые среди кода "?" и ":".

Любые правила красоты кода — они всегда имеют ограниченные показания к применению, про которые обычно в статьях, полных категорического императива «всегда делай вот так» не пишут. А потом люди читают эти статьи, и пишут
if (!condition) {
  // ...много кода...
  return 2;
}
// ...еще много кода...
return 1;

И ты такой спрашиваешь — а чё у тебя отрицание условия когда можно было и не отрицать, и блоки кода на ровном месте местами переставлены? И тебе в ответ «ну вот паттерн раннего return, туда-сюда...», и ты сидишь, и думаешь, что что-то в мире пошло не так.
0

Вы верно заметили цель употребления слова "Разумно". Я в любом случае не хочу обсуждать неразумное применение любых правил, независимо от того, призывал ли автор делать "только так" или нет. И я действительно не хочу в этот угол. Честно говоря, явно выраженные призывы постоять в углу категорических императивов вызывают у меня стойкое отторжение.


Однако, я не нашёл в тексте категорического призыва «всегда делать вот так». Я вижу цель (с которой я согласен) и предложения по решению в виде тезисов (которые никто не запрещает обсудить). Получается, что с поправкой на моё личное восприятие (в тексте этого нет, но для меня это непереопределяемое умолчание) — я получаю призыв к разумному использованию ряда практик на пути к определённой цели. Да, ограничения (типа "всегода имей в виду цель и сверяй с ней результат") не рассмотрены, но это выступление на конференции, а не учебник и не энциклопедическая статья. Выступление на конференции, как правило, не даёт ответов на все вопросы (вопросы часто задаются потом). И делать его в виде сжатого набора тезисов "что я предлагаю" (в контексте "для чего я это предлагаю") — нормально.


Чем заменить тернарник, я знаю, честно. И даже чем вложенный тернарник заменить. Вопрос мой был не о том, но возможно, я Вас изначально не понял: похоже, Вы в своём комментарии исходили из того, что всё, что может быть исполнено "неразумно", будет сделано именно так. Имея опыт оперативной техподдержки в энтерпрайзе не могу не согласиться, будет. Но тут даже явное указание на ограничения не спасает, так как всё, что может быть не прочитано… может быть не прочитано.


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

0
Тоже про ранний return подумал
ведь логичнее было бы так сделать

result = user;
if (isAdmin()) {
    result = admin;
}

return result;
0
Скажем так, ранний return нормален, если у вас несбалансированные ветки ифа — одна длинная, а другая короткая. Тогда да, выкинуть короткую ветку из else и поставить в начало с return будет вполне себе красиво и более читабельно. Но вот если что-то в этой ситуации не так, то будет классическое «но есть нюанс».

Ну да и вообще, если меня коллеги спрашивают, что вот они сидят думают час над порядком следования блоков в коде (или еще чем-то примерно настолько же очень важным) и им нужно еще мнений — мое мнение обычно выражается в «займитесь лучше чем-нибудь реально полезным» ^_^
0

А почему тогда не так?


if (isAdmin()) 
    return admin;

return user;

А если ветвей больше, то ещё нагляднее, как мне кажется:


if (isAdmin()) 
    return admin;

if (isManager()) 
    return manager;

if (isOwner()) 
    return owner;

if (isAuthorizedUser())
   return user;

return anonimUser;
0
Один return одна точка выхода из функции. Если кода мало то ваш вариант думаю допустим. Но если кода много в методе я обычно стараюсь избегать варианты где несколько return.
+2

Попытка оставить один return выльется в большую вложенность. Код внутри if/else, внутри которых ещё if/else, а там ещё ветка else, и ещё одна… только для того, чтобы завершить выполнение функции одним return. А ещё это может быть вложено в циклы. Кошмар.


Выбросив из головы странное предубеждение иметь один return вы уже неплохо упростите чтение кода и сделаете его чище.

0
уберем тернарники — вернёмся к if then else и временным переменным


Сходу не нашел, где именно, но в свое время читал, что функция, где идет широкое ветвление, берет на себя слишком мно��о, и ее желательно разбить на несколько, выполняющих что-то одно. И в этом смысле вложенные if/else и тернарные операторы действительно нужно стараться избегать, т.е. чем их меньше, тем лучше.
+1
Стремление разбивать код тоже крайне легко довести до абсурда, и получить 1000 разных функций в 100 разных файлах на пару строк каждая, и без всякой вложенности. Будет ли это читать легче одного монстро-метода на 1000 строк? Сомневаюсь, скорее всего будет примерно одинаково плохо.

Соль тут как раз в слове «разумно», прозвучавшем в комментариях выше — когда подходы применяются разумно, то есть, по совокупности доводов, а не просто «так будет читаемее и точка» — тогда всё нормально. Например, когда из процессов внутри функции можно без труда выделить подпроцессы (это обычно тогда, когда вы без труда и фантазий можете поименовать выделенную функцию, и у ревьюверов от её имени не случается WTF) — тогда их выделить может быть даже и нужно. По крайней мере это более сильный аргумент для выделения, чем «всё широкое ветвление надо разбить!!!1!».
+3
По-моему метод-монстр на 1000 строк — это совершенная патология, а вот 1000 методов в различных классах или просто функций — это логичный процесс развития программы. Т.е. при ее росте у вас, так или иначе, все равно эти 1000 методов появятся.

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

Если они реально необходимы, а не являются разбиением ради разбиения.
Чем больше методов — тем выше вероятность в них запутаться, и тем больше мата, когда через несколько лет все эти методы станут… ЛЕГАСИ!!!
+2
А чем вам так неприятен рефакторинг приватных методов с адекватным названием, без внешних эффектов с определенным числом типизированных входных и выходных параметров? Ее, в принципе, вполне безболезненно и удалить можно, т.к. ее влияние очень легко локализуется.

По мне, так этот процесс куда более безболезненный чем копание в полотне кода, где переменная может использоваться через 200, 300 и 700 строк после ее инициализации. Вот как это рефакторить и дебажить — совершенно непонятно.
0
Тысячи методов и функций в сотнях различных классах, могут привести к таком результату:
— MVP это просто. Смотри: Из вью мы вызываем метод из презентера, а презентер дергает метод из модели, модель после обработки возвращает в презентер, а презентер возвращает во вью, и все это делается коллбеками.

— Дедка за репку, бабка за дедку… через семь калбэков позвали мышку — а она давно от багов умерла.
0
Ну, да, это проблема сильной связанности кода. С ней тоже надо бороться.
0
conferences
    .filter(someItem => someItem === 'Frontend Conf')
    .forEach(someItem => console.log('Hello' + someItem));


Это то же самое, что в императивном, но гораздо проще и понятнее.


Не совсем то же самое: вместо одной итерации будет две: первая для фильтрации, вторая — для вывода.

Может это не сильно принципиальная разница, но её стоит отметить.
+1
Вообще-то эти фрагменты кода совершенно не близки по исполнению и результату :)

Это я про символичную опечатку человека, отвыкшего от сишного for.
0
Не создавайте переменные ради переменных — это плохая идея.

Это плохая идея — почему?

Что тут увеличивает читаемость — const username = user.name?

Читаемость не уверен, что увеличивает, но вот доступ например ускорить может (если поиск идёт по цепочке прототипов, или если это вычисляемое свойство, например).

Так же это может позволить потом изменить источник значения этой переменной не перекраивая все обращения к этому полю во всей области видимости.
0
Рассуждения про скорость доступа тут не причем (формально -возможно, но это на 'похвастаться"/«помериться», когда больше не чем)

Читаемость увеличит, если (some.foo()+someOther.prop)+2 присвоить переменной с осмысленным названием -то дальше по коду это будет легче пониматься.

Про «плохость» доступа черезмногие "." — все верно, у этого есть ь название (закон Деметры), Но это скорее относится к дизайну системы. Codestyle не исправит ошибки в архитектуре
-4

После строки


Читаемость ухудшают отступы

ставлю минус статье и в карму!
Дальнейшее чтение статьи прекращается, ибо писал человек понятия не имеющий о ЧИСТОТЕ и ЧИТАЕМОСТИ кода!

+8
Кажись автор местами путает декларации и функциональщину. И ту и другую можно противопоставлять императиву, но в коде, приведенном в статье идет голимая функциональщина со словами — смотри как декларативненько. Декларативность это немногое другое. Можно с пеной у рта доказывать что функциональщина более декларативна чем императивщина (и я с этим согласен), но делая something.map или forEach вы все равно указываете ЧТО ДЕЛАТЬ что бы добиться результата, а не то КАКОЙ вы результат хотите. Конкретно вот тут:

ptor.findElement(protractor.By.id('q01_D')).click()
    .then(() => ptor.findElement(protractor.By.id('q02_C')).click())
    .then(() => ptor.findElement(protractor.By.id('q03_D')).click())
    .then(() => console.log('done'));


декларативно будет что то вроде:

Must be clicked elements with id [bla bla bla]

ptor.findElement нельзя называть декларацией — это императив, хоть и в функциональном стиле.

Мое мнение, возможно никем не разделенное и однозначно никому не навязываемое.
+4
Чаю этому господину. Так называемую «декларативность» суют сейчас к месту и не к месту.
0
Радостно, что инструменты и практики взрослой разработки продолжают приходить и в js/ на фронт.

Особенно радует порт сонара.
+2
Применить паттерн раннего return

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

const.button = createButton('id_button');
button.click();

function createButton((id') {
    element = document.createElement(‘button’);
    element.id = id;
    element.classList = ‘red’;
    document.body.appendChild(element);

    return element;
}


Очень плохо. Должно быть одно из двух. Либо переименовать функцию в createRedButtonInBody либо переписать, на что-нибудь, вроде этого (я не пишу на JS и не помню синтаксис — отсюда возможные ошибки и TODO вместо кода проверок, но смысл, думаю, понятен):

const.button = createButton('id_button', 'red', document.body);
// TODO: Check if button was created.
button.click();

function createButton(id, classList, parent) {
// TODO: Check paremeters.
    element = document.createElement(‘button’);
    element.id = id;
    element.classList = classList;
    parent.appendChild(element);

    return element;
}


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

Правило одного return пришло из Си подобных языков, где нужно было освобождать память и ресурсы при выходе из функции. Ранний return отсекает граничные случаи (например валидация), и не несёт особой смысловой нагрузки, т.е. не должен заставлять прыгать взглядом.
0

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


function makeSomeAction(items, action){
    if (items.isEmpty())
        return;
    if (action == null)
        return;

    ... какой-то код
   return;
}

Как это сделать с одним return?

+3
В декларативном стиле мы скажем: «Пожарь яичницу»

«Пожарь» — в императиве.
+1
«Нам нужна яичница с беконом и помидорчиками, в меру прожаренная»?
0

Если часто в проекте вылезают декларативные конструкции типа: someArr.map().filter().map…
То лучше смотреть в сторону библиотек вроде underscore, они позволяют собирать ленивую цепочку и на последнем шаге ее применять за один проход итерирования, а не создают на каждом шаге промежуточные массивы.

+2
Читаемость ухудшают отступы

Автор очевидно имел в виду «отсутсвие» или «неправильные» отступы. Чем грешат некоторые из примеров, кстати.
Отдельно удивляют тулзы вроде Решарпера, которые любят перенести кусок длинной строки на следующую, но с изначальным отступом. Получаем «стихи Маяковского»:
  var subsubitem = list.SelectMany(
                                   element => element.Items.Where(
                                                                  i => i.id = 5)).                                                                                                            
                                                                         First()

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

Не соглашусь. Они нормально читаемы, если их правильно оформить, а не как в статье. Пример с кодом из статьи, как бы оформил я:
arr.length > 0
  ? arr[1] === 1
      ? arr[1] = 2
      : arr[1] = 1
  : console.log('empty arr');

сразу видно, к чему какие ветки относятся. Аналогично можно отформатировать приведенный код с функциями, и он будет читаемым (я этого делать не буду, великоват для комментария). Проблема в примерах с отступами и переносом строки, а не с вложенностью.
+1
Ветки видно сразу, но сама конструкция все равно тяжеловесная и стоит разделить.
0
Разделить на что? Предложите Ваш менее тяжеловесный вариант, пожалуйста.
И да, я предпочитаю использовать тернарные операторы для выбора значения, а не выполнения дествий по веткам. То есть давайте модифицируем пример:
var data = arr.length > 0
  ? arr[1] === 1
      ? arr[1]
      : arr[1] + 5
  : -8;

А если из реальной жизни, то напишите код перевода градусов по цельсию в строку с вариантами «ноль»/«ХХ тепла»/«ХХ мороза».
+2
По поводу создания временной переменной ваши коллеги правы. Иногда для значения полученного с помощью интерфейса имеется какая-либо смысловая нагрузка в теле функции. Для того, что бы увеличить читаемость логики, есть несколько вариантов:
  • Написать комментарий с описанием логики работы функции с полученным значением;
  • Сделать функцию-обертку для интерфейсной функции с другим именем;
  • Сделать временную переменную с именем, которое отражает смысловую нагрузку полученного значения применительно к алгоритму функции;

Из всех предложенных вариантов, именно последний ведет к самому качественному коду.
0
Сначала написал длинный комментарий с критикой нескольких высказываний, но потом стер.
В целом — вкусовщина и просто провокация «делайте так, потому что мне кажется, что это более читаемо». При том, что ты смотришь и думаешь: «ну как, каааак человек может считать „после“ более читаемым, чем было „до“?»

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

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

0
То же самое, у нас такие же «правила безопасности». Вот только они далеко не везде совпадают с декларируемыми в данной статье.

Главное — единообразие, а правила могут усанавливаться по месту, в зависимости от языка, средств разработки, еще каких-то факторов, включая предпочтения команды.
0
код кодом, но компанию для работы можно было и по лучше найти
Only those users with full accounts are able to leave comments., please.