Фомин Денис#23

Open

ilearnf wants to merge 33 commits into

urfu-2016:master from

ilearnf:master

Open

Фомин Денис #23
ilearnf wants to merge 33 commits into
urfu-2016:master from
ilearnf:master

Conversation

@ilearnf

@ilearnf ilearnf commented Nov 1, 2016

Copy link

Copy Markdown

No description provided.

@ilearnf


 initial

b8d4ef5

@honest-hrundel honest-hrundel changed the title ~~(削除) Денис Фомин (削除ここまで)~~ (追記) Фомин Денис (追記ここまで)

Nov 1, 2016

@honest-hrundel

honest-hrundel commented Nov 1, 2016

Copy link

Copy Markdown

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

@ilearnf


 lint

4cb855a

@honest-hrundel

honest-hrundel commented Nov 1, 2016

Copy link

Copy Markdown

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

@ilearnf


 linted

@honest-hrundel

honest-hrundel commented Nov 1, 2016

Copy link

Copy Markdown

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

@ilearnf


 test16

03f3e22

@honest-hrundel

honest-hrundel commented Nov 1, 2016

Copy link

Copy Markdown

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

@ilearnf


 test

6b12e32

@honest-hrundel

honest-hrundel commented Nov 1, 2016

Copy link

Copy Markdown

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

@ilearnf


 lint

c82ff41

@honest-hrundel

honest-hrundel commented Nov 1, 2016

Copy link

Copy Markdown

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

@ilearnf


 test

f522f4f

@honest-hrundel

honest-hrundel commented Nov 1, 2016

Copy link

Copy Markdown

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

@ilearnf


 test16

be7faab

@honest-hrundel

honest-hrundel commented Nov 1, 2016

Copy link

Copy Markdown

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

@ilearnf


 tested

8e1d77b

@honest-hrundel

honest-hrundel commented Nov 1, 2016

Copy link

Copy Markdown

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

@ilearnf


 lint

c7c8997

@honest-hrundel

honest-hrundel commented Nov 1, 2016

Copy link

Copy Markdown

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

@ilearnf


 test16

10e325f

@honest-hrundel

honest-hrundel commented Nov 1, 2016

Copy link

Copy Markdown

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

@ilearnf


 test16

c7ed3c2

@honest-hrundel

honest-hrundel commented Nov 1, 2016

Copy link

Copy Markdown

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

@ilearnf


 typo

@honest-hrundel

honest-hrundel commented Nov 1, 2016

Copy link

Copy Markdown

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

@ilearnf


 desc

cb72beb

@honest-hrundel

honest-hrundel commented Nov 1, 2016

Copy link

Copy Markdown

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

@ilearnf


 filter

e6630c6

@honest-hrundel

honest-hrundel commented Nov 10, 2016

Copy link

Copy Markdown

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

@ilearnf


 lint

03e7ea2

@honest-hrundel

honest-hrundel commented Nov 10, 2016

Copy link

Copy Markdown

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

@ilearnf


 lint

96e8d62

@honest-hrundel

honest-hrundel commented Nov 10, 2016

Copy link

Copy Markdown

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

@ilearnf


 star

43bd9a4

@honest-hrundel

honest-hrundel commented Nov 10, 2016

Copy link

Copy Markdown

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

@ilearnf


 exports call

f6c5e4b

@honest-hrundel

honest-hrundel commented Nov 10, 2016

Copy link

Copy Markdown

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

@ilearnf


 lint;

5a7064c

@honest-hrundel

honest-hrundel commented Nov 10, 2016

Copy link

Copy Markdown

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

Dodo888

Dodo888 reviewed

Nov 10, 2016

View reviewed changes

index.html Outdated

unorderedCollection = unorderedCollection.map(function (item) {

var idx = defaultCollection.indexOf(item);

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

@Dodo888 Dodo888 Nov 10, 2016

Copy link

Copy Markdown

Choose a reason for hiding this comment

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

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

@Dodo888

Dodo888 commented Nov 10, 2016

Copy link

Copy Markdown

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

@Dodo888

Dodo888 commented Nov 10, 2016

Copy link

Copy Markdown

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

@Dodo888

Dodo888 commented Nov 10, 2016

Copy link

Copy Markdown

🚀

@honest-hrundel honest-hrundel assigned msmirnov and unassigned Dodo888

Nov 10, 2016

msmirnov

msmirnov reviewed

Nov 13, 2016

View reviewed changes

@msmirnov msmirnov left a comment

Copy link

Copy Markdown

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>

@msmirnov msmirnov Nov 13, 2016

Copy link

Copy Markdown

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

@msmirnov msmirnov Nov 13, 2016

Copy link

Copy Markdown

Choose a reason for hiding this comment

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

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

@ilearnf ilearnf Nov 19, 2016

Copy link

Copy Markdown

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;

@msmirnov msmirnov Nov 13, 2016

Copy link

Copy Markdown

Choose a reason for hiding this comment

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

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

@ilearnf ilearnf Nov 19, 2016

Copy link

Copy Markdown

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;

@msmirnov msmirnov Nov 13, 2016

Copy link

Copy Markdown

Choose a reason for hiding this comment

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

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

@ilearnf ilearnf Nov 19, 2016

Copy link

Copy Markdown

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

@msmirnov msmirnov Nov 13, 2016

Copy link

Copy Markdown

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

@ilearnf ilearnf Nov 19, 2016

Copy link

Copy Markdown

Author

Choose a reason for hiding this comment

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

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

@msmirnov

msmirnov commented Nov 13, 2016

Copy link

Copy Markdown

🍅

ilearnf added 2 commits

November 19, 2016 14:46

@ilearnf


 corrections by mentor

97dcb26

@ilearnf


 deleted index

336d2f5

@honest-hrundel

honest-hrundel commented Nov 19, 2016

Copy link

Copy Markdown

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

@ilearnf


 linted

4d6a9b6

@honest-hrundel

honest-hrundel commented Nov 19, 2016

Copy link

Copy Markdown

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

@ilearnf


 lint

efa3852

@honest-hrundel

honest-hrundel commented Nov 19, 2016

Copy link

Copy Markdown

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

@ilearnf


 reduce

9e2b4e1

@honest-hrundel

honest-hrundel commented Nov 19, 2016

Copy link

Copy Markdown

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

msmirnov

msmirnov reviewed

Nov 20, 2016

View reviewed changes

lego.js

return previous.concat(current);

}, []);

}

function cloneObject(obj) {

@msmirnov msmirnov Nov 20, 2016

Copy link

Copy Markdown

Choose a reason for hiding this comment

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

Почему бы в этой функции тоже не использовать reduce?

lego.js

return clonedObject;

}

function getIntersection(listOfArrays) {

if (listOfArrays.length === 0) {

@msmirnov msmirnov Nov 20, 2016

Copy link

Copy Markdown

Choose a reason for hiding this comment

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

=== не нужно

lego.js

queryType: QUERY_TYPES.FORMAT,

query: function (collection) {

var formatted = [];

collection.forEach (function (item) {

@msmirnov msmirnov Nov 20, 2016

Copy link

Copy Markdown

Choose a reason for hiding this comment

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

Лишний пробел. Плюс тут тоже можно reduce.

lego.js

exports.sortBy = function (property, order) {

console.info(property, order);

var SORT_DIRECTION = {

@msmirnov msmirnov Nov 20, 2016

Copy link

Copy Markdown

Choose a reason for hiding this comment

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

Не понял, зачем здесь эта константа? Учитывая, что поле ASCENDING даже не используется. Я бы избавился от неё.

lego.js

queryType: QUERY_TYPES.FORMAT,

query: function (collection) {

var formatted = [];

collection.forEach (function (item) {

@msmirnov msmirnov Nov 20, 2016

Copy link

Copy Markdown

Choose a reason for hiding this comment

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

Object.assign() нельзя

lego.js

return;

return newCollection.sort(function (a, b) {

return orderMultiplyer * (String(a[property])).localeCompare(String(b[property]));

@msmirnov msmirnov Nov 20, 2016

Copy link

Copy Markdown

Choose a reason for hiding this comment

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

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

@msmirnov

msmirnov commented Nov 20, 2016

Copy link

Copy Markdown

🍅

Labels

None yet

5 participants

@ilearnf @honest-hrundel @Dodo888 @msmirnov @meded90

Conversation

@ilearnf ilearnf commented Nov 1, 2016

Uh oh!

honest-hrundel commented Nov 1, 2016

Uh oh!

honest-hrundel commented Nov 1, 2016

Uh oh!

honest-hrundel commented Nov 1, 2016

Uh oh!

honest-hrundel commented Nov 1, 2016

Uh oh!

honest-hrundel commented Nov 1, 2016

Uh oh!

honest-hrundel commented Nov 1, 2016

Uh oh!

honest-hrundel commented Nov 1, 2016

Uh oh!

honest-hrundel commented Nov 1, 2016

Uh oh!

honest-hrundel commented Nov 1, 2016

Uh oh!

honest-hrundel commented Nov 1, 2016

Uh oh!

honest-hrundel commented Nov 1, 2016

Uh oh!

honest-hrundel commented Nov 1, 2016

Uh oh!

honest-hrundel commented Nov 1, 2016

Uh oh!

honest-hrundel commented Nov 1, 2016

Uh oh!

honest-hrundel commented Nov 10, 2016

Uh oh!

honest-hrundel commented Nov 10, 2016

Uh oh!

honest-hrundel commented Nov 10, 2016

Uh oh!

honest-hrundel commented Nov 10, 2016

Uh oh!

honest-hrundel commented Nov 10, 2016

Uh oh!

honest-hrundel commented Nov 10, 2016

Uh oh!

@Dodo888 Dodo888 Nov 10, 2016

Choose a reason for hiding this comment

Uh oh!

Dodo888 commented Nov 10, 2016

Uh oh!

Dodo888 commented Nov 10, 2016

Uh oh!

Dodo888 commented Nov 10, 2016

Uh oh!

@msmirnov msmirnov left a comment

Choose a reason for hiding this comment

Uh oh!

@msmirnov msmirnov Nov 13, 2016

Choose a reason for hiding this comment

Uh oh!

@msmirnov msmirnov Nov 13, 2016

Choose a reason for hiding this comment

Uh oh!

@ilearnf ilearnf Nov 19, 2016

Choose a reason for hiding this comment

Uh oh!

@msmirnov msmirnov Nov 13, 2016

Choose a reason for hiding this comment

Uh oh!

@ilearnf ilearnf Nov 19, 2016

Choose a reason for hiding this comment

Uh oh!

@msmirnov msmirnov Nov 13, 2016

Choose a reason for hiding this comment

Uh oh!

@ilearnf ilearnf Nov 19, 2016

Choose a reason for hiding this comment

Uh oh!

@msmirnov msmirnov Nov 13, 2016

Choose a reason for hiding this comment

Uh oh!

@ilearnf ilearnf Nov 19, 2016