-
Notifications
You must be signed in to change notification settings - Fork 70
Новоселова Наталья #57
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?
Conversation
|
🍅 Не пройден линтинг или базовые тесты |
|
🍅 Не пройден линтинг или базовые тесты |
|
🍏 Пройдено тестов 15 из 15 |
lego.js
Outdated
| exports.isStar = true; | ||
| exports.isStar = false; | ||
|
|
||
| var PRIORITET = ['filterIn', 'sortBy', 'select', 'limit', 'format']; |
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.
Прочитай гайды по оформлению, пожалуйста. Нельзя называть переменные транслитом
lego.js
Outdated
|
|
||
| var PRIORITET = ['filterIn', 'sortBy', 'select', 'limit', 'format']; | ||
|
|
||
| function copyCollection(array) { |
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.
array -- плохое название переменной
оно не отражает сути
lego.js
Outdated
|
|
||
| var PRIORITET = ['filterIn', 'sortBy', 'select', 'limit', 'format']; | ||
|
|
||
| function copyCollection(array) { |
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.
Ты тут возвращаешь значение, названия таких функций принято начинать со слова get
lego.js
Outdated
| exports.query = function (collection) { | ||
| return collection; | ||
| var copy = copyCollection(collection); | ||
| var func = [].slice.call(arguments, 1); |
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.
func тоже непонятно название.
lego.js
Outdated
| */ | ||
| exports.query = function (collection) { | ||
| return collection; | ||
| var copy = copyCollection(collection); |
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.
copy? Что можно понять по переменной, которая называется копия?
lego.js
Outdated
| exports.sortBy = function (property, order) { | ||
| console.info(property, order); | ||
| return function sortBy(collection) { | ||
| var newCollection = copyCollection(collection); |
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.
Новая коллекция? Почему она новая? Чем отличается от старой?)
lego.js
Outdated
| console.info(property, formatter); | ||
| return function format(collection) { | ||
| return collection.map(function (element) { | ||
| var copy = Object.assign({}, element); |
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.
С copy та же самая история, что и выше
|
🍅 |
|
🍏 Пройдено тестов 15 из 15 |
|
🍏 |
|
🚀 |
hrundik
left a comment
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.
🍅
lego.js
Outdated
|
|
||
| function getCopyCollection(collection) { | ||
| return collection.map(function (element) { | ||
| return Object.assign({}, element); |
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.
Object.assign -- это ES2015. Мы пока остаёмся в рамках ES5, так что эту функцию использовать нельзя
| }) | ||
| .forEach (function (query) { | ||
| copyCollection = query(copyCollection); | ||
| }); |
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.
В качестве упражнения можете этот кусочек переделать на reduce, тогда можно будет переменную copyCollection не изменять. Но тут я не настаиваю.
lego.js
Outdated
| } | ||
|
|
||
| return; | ||
| return first[property] < second[property]; |
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.
sort ожидает, что компаратор возвращает число. А у вас возвращается логическое значение. И все значения, для которых условие не выполняются, преобразуются в 0, означающее, что значения равны. Так что функция сортировки корректно у вас работать не будет.
lego.js
Outdated
| console.info(property, formatter); | ||
| return function format(collection) { | ||
| return collection.map(function (element) { | ||
| var copyCollection = Object.assign({}, element); |
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.
Object.assign нельзя использовать
|
🍅 Не пройден линтинг или базовые тесты |
|
🍅 Не пройден линтинг или базовые тесты |
|
🍅 Не пройден линтинг или базовые тесты |
|
🍅 Не пройден линтинг или базовые тесты |
|
🍅 Не пройден линтинг или базовые тесты |
|
🍅 Пройдено тестов 12 из 15 |
|
🍅 Пройдено тестов 12 из 15 |
|
🍅 Пройдено тестов 11 из 15 |
No description provided.