-
Notifications
You must be signed in to change notification settings - Fork 70
Вихарев Вячеслав #25
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Вихарев Вячеслав #25
Conversation
|
🍅 Пройдено тестов 14 из 18 |
|
🍏 Пройдено тестов 18 из 18 |
|
@chipolinka обрати внимание решено доп. задание |
|
|
||
| // Constants | ||
| // | ||
| var FUNC_ORDER = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Название не совсем корректно отображает содержимое
| ]; | ||
|
|
||
|
|
||
| // Functions-helpers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Если хочешь писать доки -- у тебя есть пример, как их оформлять)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Это не доки, это просто разделение на логические части, просто для того чтоб удобнее было ориентироваться
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Кстати, почему удалил все доки?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Мешают ориентироваться в коде
Штуки полезные, понимаю
|
|
||
| // Functions-helpers | ||
| // | ||
| var _functionSorter = function (one, another) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Можешь объяснить, почему у тебя названия функций начинаются с земли?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Это функции, которые не предполагается использовать где-то еще, кроме как в этом коде, тогда как функции из следующего блока можно использовать где-то еще
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
И по правилам название функции должно начинаться с глагола.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ну не всегда
"Правила" эти довольно условны
Кажется логичнее применять "сортировщик" к коллекции, чем применять к ней "отсортировать"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Я бы всё равно избавилась на твоем месте от земли.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Хорошо, уберу
| */ | ||
| exports.select = function () { | ||
| return; | ||
| var _functionApplyer = function (obj, f) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Зачем эта функция?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Я ее передаю как аргумент в <список функций>.map с присвоенный объектом
Так для каждой функции она будет применена к объекту
| var key = keyValuePair[0]; | ||
| var value = keyValuePair[1]; | ||
|
|
||
| var newObject = Object.assign({}, obj); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
В ES5.1 еще нет Object.assign, так что нельзя использовать готовый вариант)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Упс :)
|
В целом код 👍 , но вот есть немного замечаний) |
|
🍅 |
No description provided.