Skip to content

Conversation

@ilearnf
Copy link

@ilearnf ilearnf commented Nov 1, 2016

No description provided.

@honest-hrundel honest-hrundel changed the title Денис Фомин Фомин Денис Nov 1, 2016
@honest-hrundel
Copy link

🍅 Не пройден линтинг или базовые тесты

@honest-hrundel
Copy link

🍅 Не пройден линтинг или базовые тесты

@honest-hrundel
Copy link

🍅 Пройдено тестов 16 из 18

@honest-hrundel
Copy link

🍅 Пройдено тестов 16 из 18

@honest-hrundel
Copy link

🍅 Не пройден линтинг или базовые тесты

@honest-hrundel
Copy link

🍅 Не пройден линтинг или базовые тесты

@honest-hrundel
Copy link

🍅 Пройдено тестов 16 из 18

@honest-hrundel
Copy link

🍅 Пройдено тестов 16 из 18

@honest-hrundel
Copy link

🍅 Не пройден линтинг или базовые тесты

@honest-hrundel
Copy link

🍅 Пройдено тестов 16 из 18

@honest-hrundel
Copy link

🍅 Пройдено тестов 16 из 18

@honest-hrundel
Copy link

🍅 Пройдено тестов 16 из 18

@honest-hrundel
Copy link

🍅 Пройдено тестов 16 из 18

@honest-hrundel
Copy link

🍅 Пройдено тестов 17 из 18

@honest-hrundel
Copy link

🍅 Не пройден линтинг или базовые тесты

@honest-hrundel
Copy link

🍅 Не пройден линтинг или базовые тесты

@honest-hrundel
Copy link

🍅 Не пройден линтинг или базовые тесты

@honest-hrundel
Copy link

🍏 Пройдено тестов 15 из 15

@honest-hrundel
Copy link

🍅 Не пройден линтинг или базовые тесты

@honest-hrundel
Copy link

🍏 Пройдено тестов 15 из 15

index.html Outdated
unorderedCollection = unorderedCollection.map(function (item) {
var idx = defaultCollection.indexOf(item);

return Object.assign({ position: idx }, item);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

object.assign в данной задаче использовать нельзя. Предлагается написать свой

@Dodo888
Copy link

Dodo888 commented Nov 10, 2016

Почему не хочешь пользоваться локальным линтером? Это быстрее и удобнее

@Dodo888
Copy link

Dodo888 commented Nov 10, 2016

Остались ещё неисправленные замечания.

@Dodo888
Copy link

Dodo888 commented Nov 10, 2016

🚀

@honest-hrundel honest-hrundel assigned msmirnov and unassigned Dodo888 Nov 10, 2016
Copy link

@msmirnov msmirnov left a 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>

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);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

В данной задаче не используем Object.assign()

Copy link
Author

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;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Всё, что делается в этой функции можно реализовать с помощью .reduce()

Copy link
Author

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;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Так лучше не писать, это нечитаемый код

Copy link
Author

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) {

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

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

!listOfArrays.length. сделал.

@msmirnov
Copy link

🍅

@honest-hrundel
Copy link

🍅 Не пройден линтинг или базовые тесты

@honest-hrundel
Copy link

🍅 Не пройден линтинг или базовые тесты

@honest-hrundel
Copy link

🍅 Не пройден линтинг или базовые тесты

@honest-hrundel
Copy link

🍏 Пройдено тестов 15 из 15

return previous.concat(current);
}, []);
}
function cloneObject(obj) {

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) {

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) {

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 = {

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) {

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]));

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Обзательно ли здесь приведение к строке? Кажется, localeCompare сделаем всё за нас.

@msmirnov
Copy link

🍅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants