-
Notifications
You must be signed in to change notification settings - Fork 70
Фомин Денис #23
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?
Фомин Денис #23
Conversation
|
🍅 Не пройден линтинг или базовые тесты |
|
🍅 Не пройден линтинг или базовые тесты |
|
🍅 Пройдено тестов 16 из 18 |
|
🍅 Пройдено тестов 16 из 18 |
|
🍅 Не пройден линтинг или базовые тесты |
|
🍅 Не пройден линтинг или базовые тесты |
|
🍅 Пройдено тестов 16 из 18 |
|
🍅 Пройдено тестов 16 из 18 |
|
🍅 Не пройден линтинг или базовые тесты |
|
🍅 Пройдено тестов 16 из 18 |
|
🍅 Пройдено тестов 16 из 18 |
|
🍅 Пройдено тестов 16 из 18 |
|
🍅 Пройдено тестов 16 из 18 |
|
🍅 Пройдено тестов 17 из 18 |
|
🍅 Не пройден линтинг или базовые тесты |
|
🍅 Не пройден линтинг или базовые тесты |
|
🍅 Не пройден линтинг или базовые тесты |
|
🍏 Пройдено тестов 15 из 15 |
|
🍅 Не пройден линтинг или базовые тесты |
|
🍏 Пройдено тестов 15 из 15 |
index.html
Outdated
| unorderedCollection = unorderedCollection.map(function (item) { | ||
| var idx = defaultCollection.indexOf(item); | ||
|
|
||
| return Object.assign({ position: idx }, item); |
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 в данной задаче использовать нельзя. Предлагается написать свой
|
Почему не хочешь пользоваться локальным линтером? Это быстрее и удобнее |
|
Остались ещё неисправленные замечания. |
|
🚀 |
msmirnov
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.
В целом, код непонятный и трудночитаемый. Посмотри гайды (https://github.com/urfu-2016/guides/blob/master/codestyle/js.md) и попробуй упростить.
index.html
Outdated
| @@ -0,0 +1,334 @@ | |||
| <html> | |||
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 cloneCollection(collection) { | ||
| var clonedCollection = []; | ||
| collection.forEach(function (item) { | ||
| var newItem = Object.assign({}, item); |
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()
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
| clonedCollection.push(newItem); | ||
| }); | ||
|
|
||
| return clonedCollection; |
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()
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
| return listOfArrays[0].filter(function (item) { | ||
| return listOfArrays.slice(1).some(function (otherArray) { | ||
| return otherArray.indexOf(item) !== -1; | ||
| }) || listOfArrays.length === 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.
Так лучше не писать, это нечитаемый код
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 getUnion(listOfArrays) { | ||
| if (listOfArrays.length === 0) { |
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.
Вместо listOfArrays.length === 0 можно просто писать listOfArrays.length
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.
!listOfArrays.length. сделал.
|
🍅 |
|
🍅 Не пройден линтинг или базовые тесты |
|
🍅 Не пройден линтинг или базовые тесты |
|
🍅 Не пройден линтинг или базовые тесты |
|
🍏 Пройдено тестов 15 из 15 |
| return previous.concat(current); | ||
| }, []); | ||
| } | ||
| function cloneObject(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.
Почему бы в этой функции тоже не использовать reduce?
| return clonedObject; | ||
| } | ||
| function getIntersection(listOfArrays) { | ||
| if (listOfArrays.length === 0) { |
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.
=== не нужно
| queryType: QUERY_TYPES.FORMAT, | ||
| query: function (collection) { | ||
| var formatted = []; | ||
| collection.forEach (function (item) { |
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.
| */ | ||
| exports.sortBy = function (property, order) { | ||
| console.info(property, order); | ||
| var SORT_DIRECTION = { |
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.
Не понял, зачем здесь эта константа? Учитывая, что поле ASCENDING даже не используется. Я бы избавился от неё.
| queryType: QUERY_TYPES.FORMAT, | ||
| query: function (collection) { | ||
| var formatted = []; | ||
| collection.forEach (function (item) { |
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() нельзя
|
|
||
| return; | ||
| return newCollection.sort(function (a, b) { | ||
| return orderMultiplyer * (String(a[property])).localeCompare(String(b[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.
Обзательно ли здесь приведение к строке? Кажется, localeCompare сделаем всё за нас.
|
🍅 |
No description provided.