Хабр Курсы для всех
РЕКЛАМА
Практикум, Хекслет, SkyPro, авторские курсы — собрали всех и попросили скидки. Осталось выбрать!
Хабр доступен 24/7 благодаря поддержке друзей
Комментарии 89
Захватывающая история, но когда работать-то успеваете?
Это и есть работа. Делать лучше чем было.
Понятное дело, что есть сроки и приоритеты, и не всегда успеваешь порефлексировать над тем или иным решением.
А бенчмарки прогоняли, есть разница в производительности и памяти? Это ведь на клиентских устройствах выполняется?
Да, в браузере выполняется.
Давайте честно, количество перерендеров реакта, размер бандла, количество апи запросов, объем данных, передаваемых по сети- это то, о чем нужно думать в первую очередь, как по мне. А не об этих вот микрооптимизациях в подобных алгоритмиках. Не миллионы айтемов же во входных колекциях. От силы 50 штук. Тут как ни напиши, будет хорошо работать. А вот читаемость не всегда одинаковая.
Но обладать искусством бенчмарков было бы неплохо, да.
Лично я бы в первую очередь вообще о необходимости реакта подумал. ;) А во-вторую о необходимости таких обработок на клиенте.
Но вы, я полагаю, об этих вопросах вы уже подумали, поэтому спросил по сути статьи. Просто, если второй вариант в этом плане хоть чуточку хуже, то не вижу смысла вообще тратить на него время. Кроме как для тренировки разработчика.
Хороший материал. Спасибо. Интересно. 👍
Рад, что было интересно)
Согласен, было интересно. Не понимаю, почему у многих комментаторов горит с конечного варианта функционального подхода. Как по мне, код стал лаконичнее, в отличие от первого варианта и такой не читаемой вложенности.
Я бы данные моменты выносил порой также в общении с командой, чтобы расширять кругозор вообще в написании кода) Не во всех компаниях такое есть, но круто, когда у команд выделены определенные часы, на обсуждение рефакторинга, стека технологий и прочих моментов.
Не до конца уловил вашу мысль пока..
Спорное утверждение. Мне кажется, последний вариант проще, он декларативно описывает алгоритм, скрывая все детали реализации в абстракциях. Поэтому он более читаемый.
Другой вопрос, что если захочется понять, как оно работает, то придётся погрузиться вглубь, не все полотно кода перед глазами.
Но рефакторинг и делается для того, чтобы в код можно было потом заходить, проще заходить, и проще его изменять)
Ну it-pipe, возможно, перебор будет для данной задачи. Почувствуем в ней необходимость в будущем если, дадим ей шанс.
А по поводу lodash, да, частично, а то и полностью, можно переписать на нативные вызовы.
Вот хороший ресурс по этой теме.
https://github.com/you-dont-need/You-Dont-Need-Lodash-Underscore
Но код будет, скорее всего, многословней.
Не знаю js, но решение с константами самое, на мой взгляд, удачное, можно поставить бряк и посмотреть что лежит в переменной во время дебага. А функцию pipe лучше назвать then, будет take().then().then() (взять затем flatMap затем innerJoin) и читается как предложение, а с пайп читается как "взять труба flatMap труба innerJoin", но с оператором pipe наверное самое лучшее все таки визуально, я бы выбрал его финальным если не бы не ограничения с оператором. В общем вкусовщина.
Спасибо, подумаем над предложенным переименованием.
промисы получатся...
Да, тоже заметил аналогию с ними.
Возможно просто такой нэйминг популярен для веба/бека, в мобилках я такого не встречал за 6ти летнюю разработку на Swift/ObjC/Java(недолго). Если у вас это стандартная практика, конечно лучше оставить, но у нас такое выглядело бы чужеродно. У нас популярно setup+do+expect+teardown для тестов например, для преобразований map/flatmap/reduce, то есть у нас pipe скорее всего на ревью не пропустили бы (только если это часть какой-то библиотеки то тогда ок).
различные функциональные композиции, вот и похоже
Ты чё с ума сошёл, then - метод промисов и не надо никогда называть свою кастомную хрень как системную функцию!!
Так то изначально решение намного лучше чем финальное. Т.к. оно намного понятнее и гораздо легче подается будущим изменениями логики / фиксу багов. Сам по себе код изначального решения так себе, во первых lodash тут не нужен от слова совсем, во вторых forEach надо заменить на конструкцию for(const item of items) { ... }.
Как итог, то что вы получили - полная write only шляпа и проекты с таким кодом и подходом к разработке живут очень очень очень плохо, и разработчики на таких проектах тратят в РАЗЫ(без шуток) больше времени на элементарные задачи. Учите основу основ - KISS (Keep It Simple Stupid), YAGNI (You aren't gonna need it) и код должен быть императивным, максимально простым и читаться слева направо, сверху вниз, и для того чтобы понять как он работает не должно быть распутывания.
Спасибо за ваше мнение.
Нам больше нравится функциональный стиль. Да и современный мир языков программирования наполнен им.
Взять LINQ в С#. Или Java Streams. Или те же пайпы в F# или Elixir.
Но и традиционный стиль, безусловно, имеет право на жизнь, особенно, если команде он ближе.
Всё индивидуально.
Нам больше нравится функциональный стиль.
Попрубуйте purescript =)
"Код должен быть императивным"
Ого, ничего себе заявление!
Финальный код - там даже нечему ломаться, это тот случай, когда ты написал код и сразу знаешь, что он работает именно так, как тебе нужно. Императивные решения в случае 3-4 if else уже невозможно удержать в голове. Особенно если код поддерживает не один человек, а несколько, в разные моменты времени.
Если вы адепт KISS то должны знать, что такое цикломатическая сложность и когнитивная нагрузка.
Фуф, значит я таки не одинок в своём мировоззрении.
Финальный код - там даже нечему ломаться, это тот случай, когда ты написал код и сразу знаешь, что он работает именно так, как тебе нужно
Допустим. а потом проходит месяца эдак 3-4 и нужно расширить этот функционал или изменить его, кстати вполне себе типичная ситуация.
Так вот, открываешь ты такой это место, а тебя тебя встречает вот это вот гуано функциональное, и сразу же рвотные позывы, кровотечение глазное и т.п. И давай сначала несколько часов тратить на преодоление отторжения, потом ещё N часов(жуткое отторжение виляет конкретно) на того что понять что там конкретно происходит, потом ещё время чтобы внести изменения, да ещё так, чтобы это всё и работало в рамках этого "замечательного" кода.
И вот в противовес этому традиционный императивный вариант, ну 3-5 минут ты потратил чтобы понять как оно работает, и ещё 3-5 минут чтобы изменить поведение на нужное(т.к. внося изменения в традиционном стиле, всё более чем быстро, наглядно и очевидно) и вот через 10 минут задача готова, нервы на месте, завтрак на месте, глаза видят ясно и четко.
Либо 10 минут, либо убить 1 день(в лучшем случае) с кровотечением из глаз, рвотными позывами и т.п. выбор вроде очевиден.
И самое смешное что это реально достаточно элементарная задача, и даже на ней рвотный уровень у функционального варианта очень высокий, а будь то реально сложная задача с гораздо более серьезной логикой, то если ее написать в функциональном стиле, то это всё, вообще без шансов, гарантировано write only похороненный код и проект, видел такое уже.
Императивные решения в случае 3-4 if else уже невозможно удержать в голове. Особенно если код поддерживает не один человек, а несколько, в разные моменты времени.
Серьезно? Вот прям так? Интересно получается, а как же вообще мы пишем проекты то огромные если 3-4 if'a и уже всё, в голове не удержать? Чудеса, да и только. Когда код легко читается слева на право, сверху вниз, то будь в нем хоть 20 if'ов в каждой точке ты понимаешь что происходит и при каких условия мы попадаем в ту или иную точку, ведь это прямо написано черным по белому, если это, то делаем вот так, а если вот это, то делает эдак.
Если вы адепт KISS то должны знать, что такое цикломатическая сложность и когнитивная нагрузка.
Вот именно что я это прекрасно знаю, и смотря на вариант финальный из этой статьи, хочется взять автора и просто выкинуть навсегда из этой профессии. А посмотрев на изначальный вариант, даже с учётом того, что там зачем-то воткнули lodash и вообще можно было сделать ещё получше, всё равно просто и понятно что этот код делает и что будет на выходе.
Привет, очень очень очень скептичен к финальному варианту, а как вы в середине прыгнули к к функциональному стилю так вообще чуть не стошнило. Вообщем именно в этом примере имхо у вас проблемы именно из за того что юзаете lodash, напиши функцию на чистой js по факту будет проще и читабельнее
import forEach from "lodash/forEach";
Сразу после этого отмотал статью в конец, увидел это же в результирующем коде, дальше не читал. Как минимум тимлида за то, что пропустил такое, уволить без вариантов.
Простите, не увидел того, о чем вы говорите, в финальной версии.
Сорян, я не понял сразу, что в конце изначальный код тоже продублирован. Но в результирующем коде, например, используется lodash/map. Тянуть библиотеки ради таких вещей может только тот, кто не умеет работать с массивами в js. А это настолько базовая вещь, что уже по этому очевидно, что уровень такого разработчика не то, чтобы низок, его просто нет.
У кого-то слишком много времени ). Быстрее и проще без lodash написать. А если набор данных меняется, то и поправить его будет проще. Я подобные пишу когда 100% уверен в неизменности данных и без сторонних библиотек стараюсь обходиться.
Могу ошибаться, но будто бы с решением прекрасно справились Map и Set. И lodash тут не нужен, и пайпы. Без шуток, одной из тех.задач на прошедшей неделе было выпиливание lodash из проекта)
Мужики, ну хорош! Проблема же банальная, и код должен быть таким же банальным:
const getChildrenByGroup = (childIds, parents) => {
const result = {};
for (const parent of parents) {
for (const child of parent.Children) {
if (!childIds.includes(child.Id)) continue;
const groupName = parent.GroupName || 'NoGroup';
if (!result[groupName]) result[groupName] = [];
result[groupName].push({ ...child, ParentName: parent.Name });
}
}
return result;
};
Если вложенные циклы почему-то пугают и хочется "функциональшины", то можно так. Читать и изменять такое сложнее, но хотя бы используются только стандартные методы, о которых знают все разработчики на js:
const getChildrenByGroup = (childIds, parents) => {
return parents
.flatMap(p => p.Children.map(c => ({ ...c, ParentName: p.Name, GroupName: p.GroupName || 'NoGroup'))
.filter(c => childIds.includes(c.Id))
.reduce((acc, { GroupName, ...child }) => {
// Не буду упарываться в иммутабельность аккумулятора
if (!acc[GroupName]) acc[GroupName] = [];
acc[GroupName].push(child);
return acc;
}, {});
};
Если как и вы упороться в редактирование, то можно получить даже не 7 строчек на всю функцию, а всего лишь 5. И это без ненужных импортов и кастомных функций типа pipe и innerJoin:
const getChildrenByGroup = (childIds, parents) =>
parents
.flatMap(p => p.Children.map(c => ({ ...c, ParentName: p.Name, GroupName: p.GroupName || 'NoGroup'))
.filter(c => childIds.includes(c.Id))
.reduce((acc, { GroupName, ...child }) => ({ ...acc, [GroupName]: [...(acc[GroupName] || []), child] }), {});
В статье произошло то, что постоянно наблюдаю практически в каждой компании — фронтендеры сами создают себе проблемы, а потом героически и очень муторно их решают. Видимо, так сложно принять, что фронтенд — это не рокетсайнс.
Да, это хороший вариант. Спасибо за конструктив.
Спасибо, всё описали как я пытался объяснить через flatmap/reduce, но у меня не получилось внятно это сделать. И всё верно, не нужны никакие кастомные pipe из других языков когда в стандартных библиотеках большинства языкох сейчас предостаточно функций преобразователей.
Мужики, ну хорош! Проблема же банальная, и код должен быть таким же банальным:
const getChildrenByGroup = (childIds, parents) => {
const result = {};
for (const parent of parents) {
for (const child of parent.Children) {
if (!childIds.includes(child.Id)) continue;
const groupName = parent.GroupName || 'NoGroup';
if (!result[groupName]) result[groupName] = [];
result[groupName].push({ ...child, ParentName: parent.Name });
}
}
return result;
};Всё правильно, вот единственно верный по сути адекватный вариант. Другие это уже из разряда жёстких извращений, глядя на которые кровь из глаз хлещет.
А теперь готовься поддерживать и обучать всех новых разработчиков такому функционалу. код написан и 0 строк комментариев. о поддержке можно забыть
А бенчмарки с сравнением обоих реализаций делали? Было бы интересно посмотреть.
Помимо красоты и понятности кода, тимлид должен помнить про стоимость и скорость разработки, как тут уже заметили выше тимлид выдумал проблем, героически их решал, забил на перформанс в коде, тратил время, получил дикую вкусовщину которая в итоге ещё и дорого обошлось работодателю и дорого обойдется тем, кому эту "красоту" поддерживать. Надеюсь с опытом ещё придет, а пока похоже на выдуманную переписку с самим собой. Принципы типа KISS не на ровном месте родились, фронтенд это не рокетсаенс, умение получать быстро, простой и поддерживаемый результат с вменяемым входом на поддержку - именно тот Грааль к которому стоило бы стремиться
Одна из целей ревью - расширение кругозора, рассмотрение разных подходов и вариантов решения проблемы. С этой целью мы справились, у нас было значит на это время. В этом по сути и был смысл статьи. А финальное решение мы докрутим, не сомневаетесь. А про стоимость и скорость разработки мы прекрасно осведомлены, в этом вот уж точно нет никакого рокетсайнса.
В целом, конечно, радует, когда ревью делается не для галочки, а вот так вот подробно рассматриваются варианты улучшения.
Но, честное слово, я бы убрал вот эти вот игры в "поле чудес" с вопросами вида "а попробуй догадаться, почему мне это не нравится". Респект разработчику, что он очень адекватно реагировал на такое - я видел много ситуаций, когда автор кода был не столь терпелив) В общем, на будущее совет - если Senior или Team Lead видит пути улучшения, лучше просто банально написать о них, не стоит пытаться демонстрировать собственное превосходство и играть в угадайки :)
Ну, многим людям наоборот кайфово дойти самим, и не нравится когда сразу дают готовые решения.
Да и в целом рекомендуют же развивать, задавая вопросы…
Предположу, что там заметная разница в возрасте или опыте..
Я думаю, не стоит путать ревью и решение задач на собеседованиях/литкоде. Ревьюер ведь это не тот кто знает единственно верное решение, а тот, кто делится альтернативным взглядом на решение. И предлагаемое решение абсолютно не факт, что будет единственно возможным, что и подтверждают комментарии к этой статье выше.
А по поводу "развивать" - имхо, на ревью это уже поздновато делать. Это должно происходить где-то гораздо раньше ДО ревью: на митапах, внутренних докладах, в документации и т.п. А тут видно из начального решения, что разработчик понятия не имел о предпочтениях тимлида по функциональному подходу, и на него это все свалилось как кирпич на голову прямо на ревью, когда время на решение уже потрачено, код уже написан и только ждет, чтобы его поскорее одобрили и выкатили на прод :)
Как классно!
Спасибо!
То есть факт наличия переменных с именами вас смутил, а переменная _ используемая только для export default норм? )
Да, ребят, статья немножко шире нежели то, как ее увидели многочисленные комментаторы. Она больше про подход к разработке, а, может, и к жизни. Жаль, что не всем удалось прочитать между строк. Попробую перевести и написать в явном виде, так будет проще:
Развивайтесь. Не ограничивайте себя чем-то одним, одним framework-ом, одним подходом, одним языком программирования. Смотрите по сторонам, заимствуйте идеи и концепции.
Не останавливайтесь на первом решении, хоть и работающем. Покрутите его, посмотрите на него с разных сторон, попробуйте разные варианты. Возможно, получится что-то более чистое
Изучайте платформу, для которой разрабатываете, изучайте ее API. Зачастую, что-то уже сделано до вас. Поищите. Если нет, поищите готовую библиотеку, изучите ее, изучите ее исходники. Она может вам подойти. Не подходит, roll your own
Не торопитесь. Спроектируйте свое решение на бумаге, в Visio, да, где угодно. Написание кода - последний этап в разработке решения
Не гонитесь за модой. Мир меняется.
Будьте терпимей. И не будьте такими категоричными, как в ваших комментариях
Всем удачного дня.
Развивайтесь. Не ограничивайте себя чем-то одним, одним framework-ом, одним подходом, одним языком программирования. Смотрите по сторонам, заимствуйте идеи и концепции.
Да, только не надо ударяться из крайности в крайность. Представьте, вы пришли в больницу с аппендицитом, а вам вместо хирургического удаления начала давать всякий отвары на травках и танцевать возле вас с бубном. Это примерная аналогия традиционного подхода и функционального.
Не останавливайтесь на первом решении, хоть и работающем. Покрутите его, посмотрите на него с разных сторон, попробуйте разные варианты. Возможно, получится что-то более чистое
Ага, действительно, ваше "решение" конечно капец какое "чисто" и "прекрасное". Всё что нужно было с делать с изначальный вариантом - выкинуть lodash и forEach заменить на конструкцию for(const item of items) { ... } и в принципе всё. Прекрасное, максимально очевидное, наглядное, понятное и чистое решение. А не write only и не дай боже к нему прикоснуться в дальнейшем хоть раз.
Да, ребят, статья немножко шире нежели то, как ее увидели многочисленные комментаторы.
Чего уж там, всё предельно понятно, вы как диктатор настаиваете на функциональщине в проекте не думая больше и преподносите это как взгляд со стороны и чисто решение. А как услышали слова критики, так сразу пошли оправдания и т.п., увод от темы, что это не код говеный, а подход к жизни такой, да и вообще вы ничего не понимаете идиоты которые пишут очевидный и простой код, который в любой момент и в любое время читается сверху вниз слева направо.
Будьте терпимей. И не будьте такими категоричными, как в ваших комментариях
У всего есть пределы, одно дело вместо for(const item of items) { ... } использовать forEach да ещё и из lodash или переменную назвать, ну или даже вместо await apiCall() использоваться apiCall().then(...) это ещё не так режет глаза и восприятие, хотя уже воспринимается как дичь, а другое дело вот это вот:
import groupBy from "lodash/groupBy";
import mapValues from "lodash/mapValues";
import innerJoin from "./innerJoin";
import take from "./pipe";
const _ = (childIds, parents) =>
take(parents)
.pipe($ => $.flatMap(e => e.Children.map(a => ({ Parent: e, Child: a }))))
.pipe($ => innerJoin($, x => x.Child.Id, childIds, x => x, x => x))
.pipe($ => groupBy($, x => x.Parent.Group?.Name || "NoGroup"))
.pipe($ => mapValues($, o => o.map(x => ({ ...x.Child, ParentName: x.Parent.Name }))))
.result();
export default _;и вот это
const _ = (childIds, parents) => {
const set = new Set(childIds);
const data = parents
.flatMap(e => e.Children.map(x => ({ ...x, ParentName: e.Name, Group: e.Group?.Name })))
.filter(e => set.has(e.Id));
return Object.groupBy(data, e => e.Group || "NoGroup");
};
export default _;
Тут уж извините, но такое вообще не в какое ворота не лезет. Это настолько же противоестественно и возмутительно, как лечение насморка кровопусканием. А вы это преподносите под соусом мол вот смотрите после моих наводящих вопросов разработчик пришел к такому "прекрасному" и "чистому коду", теперь он познал дзен и "идеальный" write only once код, всем бы такой код в проекты(чтобы проекты были мертворожденными).
Вот же:
const getChildrenByGroup = (childIds, parents) => {
const result = {};
for (const parent of parents) {
for (const child of parent.Children) {
if (!childIds.includes(child.Id)) continue;
const groupName = parent.GroupName || 'NoGroup';
if (!result[groupName]) result[groupName] = [];
result[groupName].push({ ...child, ParentName: parent.Name });
}
}
return result;
};Давайте, вот конкретно тут плохо? Именно реально плохо, а то ещё скажите for внутри for это плохо, но это же смешно, т.к. в конкретном случае это самое оптимальное решение.
И на самое интересно я оставил замеры. все поленились их сделать, а я нет)
Вот они - https://stackblitz.com/edit/vitejs-vite-hosyl5?file=main.js&terminal=dev
"Грязный", "Противный", "Фу фу фу" традиционный подход в среднем в 10-20 раз быстрее "чистого" кода. Такие дела.
Успокойтесь уже, идите поработайте.
Успокойтесь уже, идите поработайте.
Это всё что вы можете сказать? То что ваш вариант в 20 раз медленнее работает чем традиционный и хуже читаемый/хуже воспринимаемый (и это только на супер простецком примере) это проблемы инопланетян и тех кто придет работать сюда после вас? А вы здесь и сейчас write-only once код написали и умыли руки. "Интересный" подход.
Не вижу смысла продолжать с вами диалог, простите.
Если вы читали, в чем у меня сомнения, то написано же, что мы не ставили цели поднять перформанс. Цель была улучшить читаемость. Извините, но метрика цикломатической сложности у последнего варианта лучше.
Цикломатическая сложность не лучше. Вы же делаете map по children внутри flatMap'ы. В итоге получается тот же цикл внутри цикла. Потом, когда нужно пройти фильтром по всем-всем child'ам, вы точно такое же количество раз вызываете проверку на вхождение Child.id в childIds.
Магии не бывает. Как бы не было записано решение, для него в любом случае потребуется вложенный цикл и одно ветвление внутри цикла (на самом деле два ветвления, потому что для пустой группы нужно установить значение 'NoGroup')
Вы не путаете случайно понятие цикломатической сложности (cyclomatic complexity) с алгоритмической сложностью (линейная, квадратичная, логарифмическая)?
Не путаю. Если грубо, то каждый for, while и if добавляют по единице к цикломатической сложности. У вас программа может зайти в for, а может и не зайти, если не по чему итерироваться. Получается, что for — это тоже своего рода ветвление, которое добавляет дополнительный уникальный путь исполнения в программе.
Цель была улучшить читаемость
Увы, вы её не достигли. Если быть максимально честным, то как минимум >90% людей поймут значительно быстрее что делает вот этот код:
const getChildrenByGroup = (childIds, parents) => {
const result = {};
for (const parent of parents) {
for (const child of parent.Children) {
if (!childIds.includes(child.Id)) continue;
const groupName = parent.GroupName || 'NoGroup';
if (!result[groupName]) result[groupName] = [];
result[groupName].push({ ...child, ParentName: parent.Name });
}
}
return result;
};Чем вот этот:
const _ = (childIds, parents) => {
const set = new Set(childIds);
const data = parents
.flatMap(e => e.Children.map(x => ({ ...x, ParentName: e.Name, Group: e.Group?.Name })))
.filter(e => set.has(e.Id));
return Object.groupBy(data, e => e.Group || "NoGroup");
};Не смотря на то, что он занимает меньшее кол-во строк. Возможно и существуют единичные уникумы кто быстрее поймет 2ой вариант чем первый когда увидят код первый раз, но я таких за 12 лет в разработки не встречал.
Получается на выходе:
1) Вы получили решение которое в 20 раз медленнее работает и в N раз потребляет больше памяти(тут я поленился замерить).
2) Оно не фига не лучше читается/поддерживается и т.п. Но т.к. это совсем элементарный пример, то будем считать, что в лучшем случае(для единичных уникумов) примерно так же.
3) Вы потратили на это много лишнего времени.
А время самый ценный ресурс, который в нашей жизни, его нельзя вернуть, откатить, купить за деньги. Вместо этой пустой траты времени, просто сказали бы выкини lodash и всё. Освободили бы много времени и себе и этому разработчику. Это время можно потратить как на себя/на семью, так и на реализацию новых фич, фикса багов, а не на вот это вот всё.
Извините, но метрика цикломатической сложности у последнего варианта лучше.
Вы выучили новое слово (актуальное в 1976г.) и теперь пытаетесь любое извращение и любой ваш "аргумент" подвести под него и выставить это так, как будто это прям имеет хоть какое-то значение. Очнитесь, на первом месте всегда стояли и стоят здравый смысл, простота и читаемость кода, производительность, и скорость написания этого самого кода. Сегодня практически никто не пишет на ассемблере т.к. с позиции здравого смысла сильно много не потеряют в производительности, но сэкономят сильно много времени, того самого ценнейшего ресурса, который вы просто разбазариваете направо налево.
Можно пожертвовать скоростью исполнения во многих случаях в пользу реального выигрыша по времени написания кода, читаемости, поддерживаемости, ремонтнопригодности так сказать, но это не ваш случай.
Вы получается пожертвовали скоростью в 20 раз, но в замен получили код, чтобы понять который надо потратить больше времени, чем на традиционный вариант, который к тому же в 20 раз быстрее работает. очень странный обмен и жертвы. Плюс вы потратили на всё это время как своё, так и других людей, причем главное что напрасно потратили. И теперь гордитесь этим и гордо написали про это статью)
мы не ставили цели поднять перформанс
Ещё бы вы такие цели ставили, я был бы очень удивлен. У вас так сказать немного другой девиз)) Утилизируй время в пустую, властвуй и унижай разработчиков заставляя их писать противоестественный и ультра медленный код. Лучше бы вы ругали их за теранарки в теранарках, но мне почему-то кажется что для вас теранарки в теранарках это норма)
Буду с нетерпением ждать статьи от вас, чтобы поучиться мудрости так сказать.
Увы, вы её не достигли. Если быть максимально честным, то как минимум >90% людей поймут значительно быстрее что делает вот этот код
А откуда у вас статистика? Мне лично читать второй вариант значительно легче, чем первый. Как минимум, из названия функций я сразу понимаю, что они делают, в то время как цикл for не дает мне такой информации, пока я не загляну в тело. Кроме того, второй вариант кода просто идет сверху вниз, обрабатывая данные по очереди, в то время как первый имеет условия, которые влияют на поток исполнения и мутирует данные.
Кроме того, второй вариант кода просто идет сверху вниз, обрабатывая данные по очереди
А второй как? Снизу вверх и обрабатывает данные не по очереди, а в хаотичном порядке?))
в то время как первый имеет условия, которые влияют на поток исполнения
хоспадя, ещё как назовете простую вещь чтобы она казалась сложнее?)
и мутирует данные.
Иииии?
let ageType = 'young';
if (age > 60) ageType = 'old';Кошмар, мы мутировали данные! Такой говнокод точно нельзя писать!
Как насчет того, мутация данных удобнее, намного быстрее и намного меньше оперативной памяти съедает? Или это всё не важно, важна типичная мнимая отмазка, что якобы ты что-то мутировал и всё, всё сломалось. Ну пожалуй если ты вообще не в адеквате и не ведаешь что делаешь, то да, но этот "довод" в 99.9999% не состоятельный.
А откуда у вас статистика?
Да так, за 12 лет перевидал огромное кол-во проектов и разработчиков и знаю какой код для большинства людей быстрее и лучше воспринимается(кроме индивидуумов конечно). И не забывайте, вариант 1 и вариант 2, это варианты очень простой логики, когда речь заходит о реальной логике которая посложнее чем 2 + 2, то функциональщина становится невосприимчивым write only once кодом для для авторов этого кода, и понять что в уже в таком коде происходит куда сложнее чем в этом банальном примере. Понятное дело что с точки зрения кода такого рода вещи воспринимаются легко всеми
const filteredResult = items.map(item => item.age).filter(age => age >= 18);т.к. это тоже элементарный пример, но он всё равно значительно медленнее императивного аналога и жрёт памяти сильно больше, и когда мы оперируем исключительно очень малым объемом данных, то разницы нет, я предпочти например для такого кейса вариант с map => filter, т.к. читаемость не страдает, а экономия на ресурсах тут мизенрая, но если данных не мало и весь код соткан из такой функиональной фигни(я имею ввиду не этот детский сад аля map => filter, а конкретные такие цепочки), то суммарно приложение начинает тормозить и есть оперативку как не в себя.
А второй как? Снизу вверх и обрабатывает данные не по очереди, а в хаотичном порядке?))
Вы наверное имели ввиду первый. Если так, то да, он буквально перепрыгивает в начало цикла пока итерируется по данным, тч да исполнение тут не сверху вниз. Кроме того есть еще ключевое слово continue, которое так же заставляет сделать переход в начало цикла. И из-за всего этого опять же, я не могу знать на каждой строчке кода какое состояние имеют данные. Я только знаю, что цикл еще не окончен. Все. Из-за этого с этим кодом дальше и работать будет сложнее, но это уже тема отдельная.
хоспадя, ещё как назовете простую вещь чтобы она казалась сложнее?)
Ну так они из-за этого становится сложнее, тк надо держать доп условия выполнения в голове.
Кошмар, мы мутировали данные! Такой говнокод точно нельзя писать!
Я не давал оценку "можно"/"нельзя" к коду, ну и опять же, вы вырезаете из контекста. Сама мутация не ведет к усложнению, но когда у нас есть участок нелинейного кода который множество раз исполняется, то это уже будет усложнять чтение кода. Опять же, спор не о том сложный или легкий код для чтения, а какой код легче читать. Это разные темы.
Как насчет того, мутация данных удобнее, намного быстрее и намного меньше оперативной памяти съедает? Или это всё не важно, важна типичная мнимая отмазка
Во-первых, вы уже выдумываете темы, мы не спорили какой код лучше, мы спорили какой код лучше читается. Ну и то что мутация удобней крайне спорная тема, я описал почему в данном случае это усложняет чтение.
Ну пожалуй если ты вообще не в адеквате и не ведаешь что делаешь, то да, но этот "довод" в 99.9999% не состоятельный.
Этот тезис не относится конечно к теме спора, но это литерали "чтобы не совершать ошибки, не совершай ошибки".
Да так, за 12 лет перевидал огромное кол-во проектов и разработчиков и знаю какой код для большинства людей быстрее и лучше воспринимается(кроме индивидуумов конечно).
Нет, вы знаете какой код воспринимается легче в вашем окружении(и то даже это под вопросом). А ваше окружение дай бог 1% от всех разработчиков пишущих на этом языке.
И не забывайте, вариант 1 и вариант 2, это варианты очень простой логики, когда речь заходит о реальной логике которая посложнее чем 2 + 2,
Ну вот тогда и давайте примеры посложнее. Правда даже в данном случае, при модификации первого варианта шанс допустить ошибку больше, банально из-за нелинейности обработки данных.
т.к. это тоже элементарный пример, но он всё равно значительно медленнее императивного аналога и жрёт памяти сильно больше, и когда мы оперируем исключительно очень малым объемом данных, то разницы нет, я предпочти например для такого кейса вариант с map => filter, т.к. читаемость не страдает, а экономия на ресурсах тут мизенрая, но если данных не мало и весь код соткан из такой функиональной фигни(я имею ввиду не этот детский сад аля map => filter, а конкретные такие цепочки), то суммарно приложение начинает тормозить и есть оперативку как не в себя
Я искренни рад, что вы думаете о производительности. Но спор был о другом.
Неожиданное возвращение к теме, мне стало интересно сможет ли оптимизировать данный код js рантайм bun, и насколько он по скорости сравняется с первой версией данной функции.
По итогу, первая функция не соответствует второй, тк в итоговом результате должно быть поле Group, что не выполняется в первом варианте. Мало говорящая деталь, но забавная учитывая тезис о том, что данный код легче читать и рефакторить.
Но забавней оказался факт, что функция которая заявляется как самая медленная, работает быстрее, чем та что заявляется оптимизированной.
Я использовал библиотеку benny для замеров, так и онлайн сервисы. И везде вариант функции flatMap, filter и groupBy оказывался быстрее.
Тесты делал на ноутбуке с процессором Intel(R) Core(TM) i5-6200U и 16гб оперативной памяти
Но забавней оказался факт, что функция которая заявляется как самая медленная, работает быстрее, чем та что заявляется оптимизированной. И везде вариант функции flatMap, filter и groupBy оказывался быстрее.
Видите ли, в отличии от вас, я не заявляю, а показываю и подтверждаю приводя ссылку с кодом, по которой может перейти любой и воспроизвести, а вы сейчас просто сделали утверждение у которого 0% подтверждения. С таким же успехом вы можете сказать что - "Земля плоская, я проверил. "
Кидайте ссылку на repl где ваши результаты подтверждаются performance.now()
Вот ещё раз дублирую ссылку с замером.
https://stackblitz.com/edit/vitejs-vite-hosyl5?file=main.js&terminal=dev
Эм, простите, а вы нарочно взяли малое кол-во элементов childs? Просто если их увеличивать, то производительность резко падает вниз. Причем на большом кол-ве элементов, как вы и хотели, где это важно.
И возвращаясь к читаемости кода, вы 3 раза так или иначе правили функцию, но который раз оставляете
if (!childIds.includes(child.Id)) continue;Из-за которой, у вашего решения производительность ЗНАЧИТЕЛЬНО хуже на большом кол-ве данных чем у "freakSolution", о которых вы говорили.
Причем опять же, из-за большого кол-ва визуального шума, эта деталь сразу в глаза может и не броситься.
https://stackblitz.com/edit/vitejs-vite-duy2xi?file=main.js&terminal=dev
Эм, простите, а вы нарочно взяли малое кол-во элементов childs?
У меня там 1000 parent и каждого 1000 child и того 1млн элементов, реальной жизнью не пахнет так то, в 99.99% случаев в такого рода задачах элементов значительно меньше. Этого разве мало?
Просто если их увеличивать, то производительность резко падает вниз.
Правда?) А могло быть как-то иначе?)
И возвращаясь к читаемости кода, вы 3 раза так или иначе правили функцию
Я ее не трогал, я просто взял как есть. У меня даже не было цели выжимать производительность, а сравнить тупо в лоб. Поэтому я и не пытался ничего оптимизировать. Вот:
Из-за которой, у вашего решения производительность ЗНАЧИТЕЛЬНО хуже на большом кол-ве данных чем у "freakSolution"
Вот запустил по вашей ссылке, производительность всё равно выше. Но вообще для чистоты тестов нужно сделать так
https://stackblitz.com/edit/vitejs-vite-smsi3c?file=main.js&terminal=dev
А теперь да, делаем простейшую оптимизацию и самое очевидное как вы говорили, заменить
if (!childIds.includes(child.Id)) continue;
И получаем:
https://stackblitz.com/edit/vitejs-vite-c7yd3i?file=main.js&terminal=dev
Можно пойти ещё дальше, просто мутировать child'a, т.к. в реальном кейсе добавление к нему ParentName дает ровно 0 негативных эффектов, а вот к производительности дает много
Получаем ещё ускорение ~30%
https://stackblitz.com/edit/vitejs-vite-puaqos?file=main.js&terminal=dev
Итого приложив 0% усилий и 30 секунд времени ускорили getChildrenByGroup в ~4 раза. С нулевым ущербом читаемости.
Нет смысла пытаться отрицать очевидные преимущества мутабильности и императивном подходе в качестве кода, его читаемости, производительности и потреблению оперативной памяти.
Если привести код к идентичному алгоритму, то по производительности одинаково. Разница в пределах погрешности:
https://stackblitz.com/edit/vitejs-vite-w8s7p9d6?file=main.js,javascript.svg&terminal=dev
Нет смысла пытаться отрицать очевидные преимущества мутабильности и императивном подходе в качестве кода, его читаемости, производительности и потреблению оперативной памяти.
Так же как и нет смысла отрицать очевидные недостатки мутабельности в качестве источника неочевидных ошибок в программе.
Во втором варианте Вы мутируете parents. А ведь оно могло где-то еще использоваться в другой части программы.
Возможно и существуют единичные уникумы кто быстрее поймет 2ой вариант чем первый когда увидят код первый раз, но я таких за 12 лет в разработки не встречал.
Берём код, предложенный выше @Telichkin , немного поправим его:
"use strict";
const selectAllChildren = (data) =>
data.flatMap((p) =>
p.Children.map(
(c) => ({ ...c, ParentName: p.Name, Group: p.Group.Name } || "NoGroup")
)
);
const selectFromList = (childIds) => (data) =>
data.filter((c) => childIds.includes(c.Id));
const grouping = (data) =>
data.reduce(
(acc, { Group, ...child }) => ({
...acc,
[Group]: [...(acc[Group] || []), child],
}),
{}
);
const DO =
(...fns) =>
(data) =>
fns.flat().reduce((output, fn) => fn(output), data);
const testList = [
{
Name: "Parent1",
Group: { Name: "Group1" },
Children: [{ Id: 1, Name: "Child1" }],
},
{
Name: "Parent2",
Group: { Name: "Group2" },
Children: [
{ Id: 2, Name: "Child2" },
{ Id: 3, Name: "Child3" },
],
},
{
Name: "Parent3",
Group: { Name: "Group1" },
Children: [
{ Id: 4, Name: "Child4" },
{ Id: 5, Name: "Child5" },
],
},
];
let getChildrenByGroup = DO(selectAllChildren);
console.log("result1:");
console.log(getChildrenByGroup(testList));
getChildrenByGroup = DO(selectAllChildren, selectFromList([1, 2, 5]));
console.log("result2:");
console.log(getChildrenByGroup(testList));
getChildrenByGroup = DO(selectAllChildren, selectFromList([1, 2, 5]), grouping);
console.log("result3:");
console.log(getChildrenByGroup(testList));
getChildrenByGroup = DO(selectAllChildren, selectFromList([7]), grouping);
console.log("result4:");
console.log(getChildrenByGroup(testList));
Что может быть проще для понимания?:
getChildrenByGroup = DO(selectAllChildren, selectFromList([1, 2, 5]), grouping);Да, можно запрятать методы массива, если они так напрягают.
эх... мне бы такого ревьюрера!!!
Зарегистрируйтесь на Хабре, чтобы оставить комментарий
Один день из жизни JavaScript разработчика и его техлида