Skip to content

Титова Наталья#53

Open
Naturin wants to merge 4 commits intourfu-2017:masterfrom
Naturin:master
Open

Титова Наталья#53
Naturin wants to merge 4 commits intourfu-2017:masterfrom
Naturin:master

Conversation

@Naturin
Copy link
Copy Markdown

@Naturin Naturin commented Oct 18, 2017

No description provided.

@honest-hrundel honest-hrundel changed the title v.1 Титова Наталья Oct 18, 2017
@honest-hrundel
Copy link
Copy Markdown

🍅 Пройдено тестов 6 из 19

@honest-hrundel
Copy link
Copy Markdown

🍅 Пройдено тестов 6 из 19

@honest-hrundel
Copy link
Copy Markdown

🍅 Пройдено тестов 12 из 19

@honest-hrundel
Copy link
Copy Markdown

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

Copy link
Copy Markdown

@savichev-igor savichev-igor left a comment

Choose a reason for hiding this comment

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

Читать код сложно, постарайся внести побольше семантики и избавиться от этих индексов, посмотри в сторону использования объектов и свойств

Добавь где-то JSDoc или лаконичные комментарии, чтобы объяснить что происходит

🍅

var MINUTES_IN_HOUR = 60;
var HOUR_IN_DAY = 24;
var TOTAL_TIME_LINE = WEEK_DAYS.length * HOUR_IN_DAY * MINUTES_IN_HOUR;
// var TRY_LATER_MINUTES = 30;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Такого рода комментарии лучше удалять

.replace('%MM', (minutes < 10 ? '0' : '') + minutes);
}

// /**
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

???


function calcGangPartyFreeTime(scheduleArr) {
var result = [];
for (let personName in scheduleArr) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

forEach?

continue;
}
var personResult = [];
for (let i = 0; i < scheduleArr[personName].length; i++) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Используешь где-то let, но при этом не используешь const для констант

return hours * MINUTES_IN_HOUR + Number(format[3]);
}

function busyTimeToFreeTime(timeArr) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Как-то много индексов и каких-то проверок в этом методе, постарайся хотя бы сделать, чтобы было обращение к свойствам, а не индексам, ибо вообще сложно разобраться что к чему здесь

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

За 2 минуты так и не смог понять, что делает этот метод

function busyTimeToFreeTime(timeArr) {
var result = [];
for (let i = 0; i <= timeArr.length; i++) {
if (i === 0) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Почему такое исключение для первой итерации?

function getBankWorkingTime(timeArr) {
var result = [];
for (let i = 0; i < WEEK_DAYS.length; i++) {
result.push([stringToInt(WEEK_DAYS[i] + ' ' + timeArr.from),
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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


function getBankWorkingTime(timeArr) {
var result = [];
for (let i = 0; i < WEEK_DAYS.length; i++) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Только не for)

var arr2 = compare ? arrays[0] : arrays[1];
for (let i = 0; i < arr1.length; i++) {
for (let k = 0; k < arr2.length; k++) {
// if (arr1[i][0] >= arr2[k][1] || arr1[i][1] <= arr2[k][0]) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Удалить

}

function checkTimeInterval(arr1, arr2, newArr) {
if (arr1[0] >= arr2[1] || arr1[1] <= arr2[0]) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Индексы - сложно и непонятно

@savichev-igor savichev-igor dismissed their stale review October 19, 2017 10:31

Поторопился

@savichev-igor
Copy link
Copy Markdown

savichev-igor commented Oct 19, 2017

Почему-то в почту стали падать уведомления про твой PR, думал что уже можно проверять

Поздно увидел, что тесты ещё не все пройдены

Можешь уже опираться на моё частичное ревью)

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants