Истоки
Когда-то давно JavaScript разработчики очень любили использовать анонимные функции (собственно и сейчас многие их любят использовать), и всё бы ничего, и код становится короче, и не надо придумывать очередное название для функции, но рано или поздно это превращается в “водопад функций” прочитать который вы сможете разве только в хорошем IDE и то немного поломав голову.
Вот вам один пример:
Пример 1:
$http.request('GET', '/api').then(function (data) {
return data.arr.map(function (item) {
item.props.filter(function (prop) {
return !!prop;
}).map(function (prop) {
prop.a = prop.a * 2;
return prop;
})
}).filter(function (item) {
return item.props.some(function (prop) {
return prop.a > 10;
});
});
}, function (error) {
throw new TypeError(error);
});Это очень простой пример, в моей практике я встречал “монстров” на 100 — 200, а то и 300+ строк кода, которые приходилось рефакторить, прежде чем понять что за магию они делают.
В своё время переход на “шаблон функционального дизайна”, в котором каждая функция объявляется отдельно и используются только ссылки на них сделал код красивым, аккуратным и самое главное легко читаемым, что очень важно, когда вы работаете с ним не один.
Этот пример, например, преобретает следующий вид:
Пример 2:
$http.request('GET', '/api').then(requestSuccessHandler, requestErrorHandler);
function requestSuccessHandler(result) {
return result.arr
.map(transformResultArr)
.filter(filterResultArr);
function transformResultArr(item) {
item.props
.filter(checkProperty)
.map(transformProperty)
}
function filterResultArr(item) {
return item.props.some(filterProperty);
}
function checkProperty(prop) {
return !!prop;
}
function transformProperty(prop) {
prop.a = prop.a * 2;
return prop;
}
function filterProperty(prop) {
return prop.a > 10;
}
}
function requestErrorHandler(error) {
throw new TypeError(error);
}Кода хоть и больше, но зато с таким вариантом проще работать, он легче читается и дебажится.
Но всё хорошее длилось не долго.
Стрелочные функции
Прежде чем перейдём к проблеме, давайте посмотрим что-же такое стрелочный функции и что они нам дали со своим приходом.
Стрелочный функции это короткая запись анонимных функций. В принципе это всё, но на практике это оказывается очень полезно. Например, если вам надо выбрать из массива все элементы имеющие поле id. Раньше это было бы записано так:
Пример 3:
arr.filter(function(item) {
return !!item.id;
});Или так:
Пример 4:
arr.filter(filterID);
function filterID(item) {
return !!item.id;
}Но со стрелочными функциями эта запись станет неприлично короткой:
Пример 5:
arr.filter(item => !!item.id);Вроде бы всё великолепно. И да, здесь я согласен, что стрелочные функции сделали код гораздо более лаконичным, не навредив при этом его читаемости, и во всех простых случая это будет так. И я уж было подумал, что нашёлся новый стаилгайд полный анонимных функций, но не тут то было.
Проблема появляется не сразу.
Стрелки во всём
Недавно я перешёл на новый проект, в котором с самого начала активно используется ECMAScript 2015, и как водится везде по максимуму используются новые фичи языка. Самой заметной из них является повсеместное использование стрелочных функций. И всё было бы хорошо, если бы это не были водопады функций на 100+ строк и 4+ уровней вложенности.
Для наглядности вернёмся к первому примеру, в этом проекте он бы выглядел примерно так:
Пример 5:
$http.request('GET', '/api').then(data => {
return data.arr.map(item => {
item.props.filter(prop => !!prop)
.map(prop => {
prop.a = prop.a * 2;
return prop;
})
}).filter(item => item.props.some(prop => prop.a > 10));
}, error => {
throw new TypeError(error)
});В общем то он стал короче (и я сам кстати люблю, когда код короткий). Местами он стал понятней. Но это только если его сравнивать с первым примером. На практике такой код вырастает в огромных не читаемых монстров, которые быстро пишутся, но потом часами дебажатся и дорабатываются.
Но я не говорю, что использовать стрелки плохо, просто их надо использовать в меру.
Как же всё сделать лучше?
На пути создания хороших практик с ECMAScript 2015 мы будем ещё долго спотыкаться и падать, но для начала давайте начнём объединять лучшие старые и новые практики.
Например соединим примеры 2 и 5:
Пример 6:
$http.request('GET', '/api').then(requestSuccessHandler, requestErrorHandler);
function requestSuccessHandler(result) {
return result.arr
.map(transformResultArr)
.filter(filterResultArr);
function transformResultArr(item) {
item.props
.filter(prop => !!prop)
.map(transformProperty)
}
function filterResultArr(item) {
return item.props.some(prop => prop.a > 10);
}
function transformProperty(prop) {
prop.a = prop.a * 2;
return prop;
}
}
function requestErrorHandler(error) {
throw new TypeError(error);
}Если использовать стрелочные функции для простых одно строчных функций, то можно избежать создания огромного количества маленьких функций, не навредив при этом читаемости кода, но если функция сложнее одного действия, то чаще лучше вынести её отдельно, чтобы её проще было при необходимости задебажить и любой другой разработчик легко бы понял, что происходит на этом участке кода.
Комментарии (48)
vintage
11.07.2016 13:24+1Как насчёт такого варианта?
class $my_app { itemsRaw() { return $mol_http.resource( '/api' ).json().arr } itemsNormal() { var items = this.itemsRaw() items.forEach( item => { if( !item.prop ) return item.prop *= 2 } ) return items } itemsFiltered() { return this.itemsNormal().filter( item => { return item.props.some( prop => prop.a > 10 ) } ) } }vintage
11.07.2016 13:29Точнее так:
class $my_app { itemsRaw() { return $mol_http.resource( '/api' ).json().arr } itemsNormal() { var items = this.itemsRaw() items.forEach( item => item.prop.forEach( prop => { if( !prop ) return prop.a *= 2 } ) ) return items } itemsFiltered() { return this.itemsNormal().filter( item => { return item.props.some( prop => { return prop.a > 10 } ) } ) } }vintage
11.07.2016 13:43Или даже так:
class $my_app { itemsFiltered() { return this.itemsNormal().filter( item => this.itemCheck( item ) ) } itemCheck( item ) { return item.props.some( prop => prop.a > 10 ) } itemsNormal() { return this.itemsRaw().map( item => this.itemNormalize( item ) ) } itemNormalize( item ) { item.prop.forEach( prop => { if( !prop ) return prop.a *= 2 } ) return item } itemsRaw() { return $mol_http.resource( '/api' ).json().arr } }
vtrushin
11.07.2016 13:52+1Зачем создавать класс, если здесь нет локальных свойств? Просто чтоб сгруппировать несколько функций? Да и методы должны быть глаголами
vintage
11.07.2016 14:05+2Например, за этим:
class $my_app_prefixed extends $my_app { itemsNormal() { return [ this.itemPrefix() ].concat( super.itemsNormal() ) } itemPrefix() { return { prop : [ { a : 10 } ] } } }
Это процедуры (itemNormalize) и конвертеры (itemCheck) должны быть глаголами. Геттеры же (itemsFiltered, itemsNormalized, itemsRaw) вполне могут именоваться и существительными.
dom1n1k
11.07.2016 13:34+6Мое мнение: стрелочные функции хороши тогда, когда они укладываются в одно выражение (арифметическое или логическое).
Ну и конечно в случае, если принципиально важно сохранить this.
Во всем остальном лучше традиционные.
stardust_kid
11.07.2016 14:26Ну и конечно в случае, если принципиально важно сохранить this.
Да и this во многих случаях проще передавать, как параметр.
Veikedo
11.07.2016 15:12Ничто не мешает сделать так, сохранив контекст и выделив имя функции.
get(...).then(result => processResult(resut))Gaito
11.07.2016 15:13Только по моему будет аккуратней если через bind прокинуть this
get(...).then(processResult.bind(this))vintage
11.07.2016 20:13Конкретно в данном случае можно и так, но в общем случае так лучше не делать. Иллюстрация:
'33333'.split( '' ).map( parseInt ) // [3, NaN, NaN, NaN, 3]
netgoblin
12.07.2016 00:50Скажите, а почему так получается?
surefire
12.07.2016 02:38+1Array#map— передает функции обратного вызова 3 аргумента, элемент массива, индекс и сам массив.
parseInt(string, radix);— принимает 2 аргумента строку и целое число в диапазоне между 2 и 36, представляющее собой основание системы счисления.
parseInt(3, 0) // = 3 parseInt(3, 1) // = NaN parseInt(3, 2) // = NaN parseInt(3, 3) // = NaN parseInt(3, 4) // = 3
Laney1
11.07.2016 15:13+1можно писать так: let myFunc = (args) => {… }
и имя есть, и this сохраняется.Gaito
11.07.2016 15:16+2Да, но в «боевом» коде могут быть снова проблемы с читаемостью. Чаще всего все функции лучше объявлять в конце кода через function, а не как переменную. И почти никогда нет проблемы прокинуть контекст через call, bind или apply.
ganqqwerty
11.07.2016 15:01+1это правда, стрелочки начинают читаться намного хуже после двух строк кода в теле функции
Ryppka
11.07.2016 15:31+2Помниться, про написание программ на C++ Б. Страуструп писал:,"… руководствуйтесь чувством меры, здравым смыслом и подражайте хорошим образцам...". Судя по личному опыту, это трудно. Но, если хочется выразительный язык, то без этого никак. Почему в JS должно быть иначе?
Alexey2005
13.07.2016 16:02Монструозность JS возрастает очень быстро, ещё лет десять — и он догонит C++. Особенно если кого-нибудь осенит «светлая» идея добавить в стандарт ECMAScript #define/#ifdef.
cheshir_Kat
11.07.2016 15:31Просто многие часто забывают, что новая фича синтаксиса зачастует не обеспечивает «хорошего кода» по умолчанию и начинают пихать везде, где нужно и не нужно. Пусть думают сначала, а потом пишут. К тому же обычные анонимные функции никуда не делись и продолжать их испольовать никто не заприщает и в ES6
megahertz
11.07.2016 18:13Проблему частично решает Async/Await, жаль, без babel пока нигде не работает
faiwer
11.07.2016 18:34+2А так?$http.request('GET', '/api') .then(data => { return data.arr .map(item => { return item.props .filter(prop => Boolean(prop)) .map(prop => { prop.a = prop.a * 2; return prop; }); }) .filter(item => { return item.props.some(prop => prop.a > 10); }); }) .catch(e => new TypeError(error));
Finesse
12.07.2016 14:26Поддерживаю! Отступы играют большую роль в читаемости. Такой пример более читаем, чем вариант с отдельными именнованными функциями.
StGeass
12.07.2016 16:40Про .map хорошо подметили, если не важна мутабельность, тогда уж использовать map по назначению, и тогда всё выглядит ещё более читабельно:
Вместо
.map(prop => { prop.a = prop.a * 2; return prop; });
С object spread будет
.map(prop => ({...prop, a: prop.a * 2}))
или если spread режет глаз
.map(prop => Object.assign({}, prop, {a: prop.a * 2}))
Если всё-таки важна мутабельность и сохранение цепочки, то Object.assign снова же можно заюзать
.map(prop => Object.assign(prop, {a: prop.a * 2}))
PsyHaSTe
13.07.2016 11:31Прям антипаттерн по-Мартину, ибо его позиция — вместо комментария всегда создавать новую функцию :)
faiwer
13.07.2016 12:01+1Тут проблема в том, что метод оторван от контекста. Расположен чёрт знает где и не видно окружения его вызова. Да и именование метода сильно уступает комментарию по выразительности. Если писать вот в таком вот callback-стиле, то попытка раздербанить один средней сложности вложенный кусок кода на полтора десятка методов с длинными непонятными названиями приведёт к полной дезориентации. Как собственно у автора и получилось с его
transformResultArr,transformPropertyиcheckProperty. Мусорные названия однострочных методов вырванных из контекста и расположенных в произвольном порядке. А всего то нужно было отступы правильно задать.
В общем тут палка о двух концах. Когда стрелочные анонимки жиреют и становятся сложными — их нужно выносить вовне. Или же когда речь идёт о переиспользуемости кода.
PsyHaSTe
13.07.2016 20:04Да я не говорю, что я с Мартиным согласен, чай не сразу после прочтения оного пошел в интернет разносить мудрость :) Но смысл в этих словах есть. Писать в callback-стиле вообще печально, а писать так или вот эдак лишь придает определенный оттенок горечи. Поэтому в моем основном языке есть прекрасный async/await (вроде как в JS запилили yield, но я за ним не слежу, не скажу), который весь этот коллбек-колбасу превращает в абсолютно линейный код. И даже оступов сверх обычного никаких не нужно.
faiwer
13.07.2016 20:57async/awaitкасается только асинхронного кода. Синхронные цепочки трансформации данных никуда не деваются. Хотя наверное "callback-ый ад" — это не про них :) Но приведённый в примерах код как раз о них.
faiwer
13.07.2016 21:04Забыл сказать, мне в
scalaпонравилось то, как удобно там организованы простые случаиcallback-ов:
collection .map(el => el.export()) .filter(el => el.age > 10) // => collection.map(_.export).filter(_.age > 10)
Плюс стандартная библиотека умеет большую часть насущных вещей из
lodash.js.
vintage
13.07.2016 22:23прекрасный async/await
В том, что у вас весь код усеян асинками да авайтами нет ничего прекрасного. В ноде можно писать безо всей этой ерунды:
var Future = require( 'fibers/future' ) var Fetch = require( 'fetch-promise' ) class Transport { fetch( uri ) { return Future.fromPromise( Fetch( uri ) ).wait() } fetchJSON( uri ) { return JSON.parse( this.fetch( uri ).buf ) } fetchTable( uri ) { return this.fetchJSON( uri ).data.children.map( ({ data })=> data ) } }faiwer
14.07.2016 07:24Не желаете написать обзорную статью про
fibersвnodeJS? :) Было бы интересно узнать про все подводные камни и сравнение с теми же async-promise-ми. Такие вопросы как производительность, принцип работы волокон (признаться, я вот так его и не понял), обработка ошибок, сегфолты (когда баловался — часто их наблюдал). В частности интересно, почему такой подход в nodeJS есть давно, но всё остаётся уделом одиночек?vintage
14.07.2016 10:05+3Давно хочу написать/записать, но руки не доходят.
Async функция — это фактически обёртка над генератором. Генератор — это фактически конечный автомат. А конечный автомат — это фактически switch-case с выбором ветки кода в зависимости от значения счётчика.
Волокно — это указатель на стек. В любой момент вы можете "заморозить" текущее волокно, передав управление другому (изменив указатель на другой стек), а по какому-либо событию "разморозить" и продолжить исполнение. Так как изменение указателя — тривиальная операция, волокна ещё называют "легковесными потоками". То есть они обеспечивают многопоточность даже в однопоточной среде.
Ошибки обрабатываются стандартно — через try-catch. При этом рекомендуется использовать Future (это аналог промисов, но вместо then и catch, используется wait), которые исправляют стектрейсы.
С сегфолтами не сталкивался, но расширение это довольно зрелое. А мало кем используется потому, что нет хайпа и в браузере не заведётся.
Минусы волокон:
- Это не стандарт.
- Это бинарное расширение.
- Работает только в NodeJS.
- Отладчик (во всяком случае в вебшторме) не может получить значения переменных после пробуждения волокна.
- Большее потребление памяти в сравнении с генераторами (необходимо хранить стек волокна).
Плюсы волокон:
- Это расширение платформы, а не языка.
- Асинхронность инкапсулируется в одной функции и не распространяется как вирус по всему приложению.
- Больше скорость в сравнении с генераторами. Накладные расходы только на создание и переключение волокна. Генератор даёт пенальти на каждый свой вызов.
Ну и для полноты картины: сравнение кодов с разной реализацией асинхронности.
vintage
16.07.2016 01:56+1Я там по ссылке добавил замеры времени.
- Самой быстрой, неожиданно, оказалась асинхронная версия на атомах. Видимо сказывается минимум замыканий.
- Незначительно отстаёт синхронная версия.
- С почти двухкратным отставанием идут асинхронные версии на колбэках и файберах.
- Чуть погодя — асинхронные версии на промисах и генераторах.
- И ещё в 3 раза медленней — то, что сгенерировал Babel из async/await.
vintage
16.07.2016 14:31+1Ну и совсем для полноты картины добавил примеры стек трейсов.
Информативные стектрейсы (от точки старта) лишь у синхронной версии и псевдосинхронных (волокна, атомы). Остальные же асинхронные версии обрываются на ближайшей асинхронной операции.
Cubicmeter
13.07.2016 13:18+1Комментарии всегда намного хуже отдельных методов с говорящими названиями.
faiwer
13.07.2016 15:16+1Комментарии всегда намного хуже отдельных методов с говорящими названиями.
Это чем же?
- вы дублируете говорящее_название_метода дважды (вызов + определение метода)
- вы ограничены правилами именования методов, да ещё и codeStyle-ом
- имя метода максимум 1 строка
- тело метода (в данном контексте одно-двустрочник) располагается отдельно (и возможно далеко) от места использования
- для того чтобы имя метода стало говорящим в отрыве от контекста может потребоваться намного больше слов, чем комментарию по месту
- в зависимости от области видимости может возникнуть именная коллизия
За примерами ходить не нужно — в статье их навалом. Самый глупый метод —
checkProperty. Вообще ни о чём не говорящее название. А при использовании lodash всего то потребовалось бы.compact(). По сути если стараешься писать в около-функциональном стиле, то выносить одно-двух строчные очевидные действия в отдельные методы — сильно ухудшать кодовую базу. Всё сразу становится сложным и непонятным.Cubicmeter
14.07.2016 12:15Тут я соглашусь с тем, что вам ответил PsyHaSTe — к очевидным действиям и комментарии не нужны. Они только замусоривают код. IDE показывает их серым, и глаз их игнорирует.
Если же есть несколько строк кода, выполняющих какую-то одну цельную _функцию_ с неким смыслом — они так и должны быть выделены в функцию. Где будет ее тело, не особо важно, т.к. в IDE есть переход к определению и обратно.
Отдельно остановлюсь тут — «вы дублируете говорящее_название_метода дважды (вызов + определение метода)».
Извините, но это не дублирование. Дублирование — это фразы, имеющие одинаковый смысл. Вызов и определение метода никаким дублированием, конечно, не является. А вот когда блоки кода не выносятся в функцию — это провоцирует и дублирование, и что еще хуже — написание заново кода с тем же функционалом в другом месте.faiwer
14.07.2016 13:25Где будет ее тело, не особо важно, т.к. в IDE есть переход к определению и обратно.
А толку то от этого? Вы теряете контекст. Для одно-дву-строчных методов это критично. А именно о них мы и говорим.
JCHouse
11.07.2016 19:03+2Ну правильно давайте все фигачит именными функциями:) Может все таки не зря сахар для классов сделали?
OpieOP
11.07.2016 21:12Судя по рекомендациям от инженеров Google подход с анонимными функциями еще намного медленнее, чем объявление их отдельно, т.к. при каждом запросе создается новая анонимная новая функция, если правильно все понял.
OpieOP
11.07.2016 21:18ссылка не прикрепилась https://developers.google.com/speed/articles/optimizing-javascript
kashey
11.07.2016 22:33Так и есть. А потом еще и удаляется, вызывая фризы GC. Ну и слабо оптимизируются, так как мало раз выполняются.
В этом плане обычные замыкания от «стрелочных» никак не отличаются. Но боятся этого надо когда такое создание/уничтожение имеет массовый характер.
И можно было бы сделать (и имело бы смысл) код как в примере №2 — он был бы самым быстрым и про однократной работе коде, и, особенно, при многократной.
enload
12.07.2016 00:50+1Сначала хочется отметить, что раз речь идет о функциональном подходе, то стоит в примерах стремиться и к чистоте функций. И именованные функции — это конечно, замечательно, но здесь также надо знать меру, ровно как и со стрелочными функциями.
К слову, в примере со стрелочными функциями data.arr заполнится undefined.
А приведенный пример остается достаточно понятным и читабельным как-нибудь так (дальнейшая декомпозиция зависит уже от контекста и условий задачи, тестирования и многих факторов):
const requestErrorHandler =…
const requestSuccessHandler = ({ arr }) => arr
.map(({ props }) => props
.filter(prop => !!prop)
.map(prop => ({ ...prop, a: prop.a * 2 })))
.filter(({ props }) => props
.some(({ a }) => a > 10));
$http
.request('GET', '/api')
.then(requestSuccessHandler, requestErrorHandler);enload
12.07.2016 09:22Хабр порезал разметку, код должен был выглядеть как-то так: https://jsfiddle.net/nsza9z5u/
const requestErrorHandler = … const requestSuccessHandler = ({ arr }) => arr .map(({ props }) => props .filter(prop => !!prop) .map(prop => ({ ...prop, a: prop.a * 2 }))) .filter(({ props }) => props .some(({ a }) => a > 10)); $http .request('GET', '/api') .then(requestSuccessHandler, requestErrorHandler);
kemsky
12.07.2016 23:44Мне кажется, что до тех пор пока вы используете только методы массива все нормально:
array.map().map().filter().map().etc(). Сложности начинаются когда это все в одном месте с промисами или чем-то похожим, т.е. в вашем примере достаточно вынести код трансформирующий результат в отдельный метод и всем все будет ясно, можно дополнительно написать обертку надrequestкоторая будет принимать опциональную функцию трансформер.
k12th
Ну нет. Гораздо важнее, что они сохраняют контекст (точнее, не создают свой собственный, что теоретически делает их быстрее там, где они поддерживаются нативно).
А так да, именованные функции, особенно если они длиннее 1-2 строк, проще читать и поддерживать (если они нормально названы, конечно:)).