Skip to content
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

Левшин Михаил #18

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
199 changes: 167 additions & 32 deletions robbery.js
Original file line number Diff line number Diff line change
@@ -1,49 +1,184 @@
'use strict';

/**
* Сделано задание на звездочку
* Реализовано оба метода и tryLater
*/
exports.isStar = true;

/**
* @param {Object} schedule – Расписание Банды
* @param {Number} duration - Время на ограбление в минутах
* @param {Object} workingHours – Время работы банка
* @param {String} workingHours.from – Время открытия, например, "10:00+5"
* @param {String} workingHours.to – Время закрытия, например, "18:00+5"
* @returns {Object}
*/
var DAYS_INDEXES = { 'ПН': 0, 'ВТ': 1, 'СР': 2, 'ЧТ': 3, 'ПТ': 4, 'СБ': 5, 'ВС': 6 };
var DAYS_NAMES = ['ПН', 'ВТ', 'СР', 'ЧТ', 'ПТ', 'СБ', 'ВС'];
Copy link

Choose a reason for hiding this comment

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

Это ключи предыдущего объекта

var MINUTES_IN_HOUR = 60;
var HOURS_IN_DAY = 24;
var MINUTES_IN_DAY = HOURS_IN_DAY * MINUTES_IN_HOUR;
var MINUTES_TO_START_LATER = 30;

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.

мешает

Choose a reason for hiding this comment

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

Но они же несут полезную информацию!

var DAYS_TO_HACK = 3;


function TimeInterval(start, end) {
this.start = start;
this.end = end;

this.getLength = function () {
Copy link

Choose a reason for hiding this comment

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

start и end известны на этапе создания объекта, почему length нужно вычислять, а нельзя сделать полем?

return this.end - this.start;
};

this.getNextDay = function () {
return new TimeInterval(this.start + MINUTES_IN_DAY, this.end + MINUTES_IN_DAY);
};

this.isTimeInInterval = function (time) {
return this.start <= time && time <= this.end;
};
}

function isTimeInIntervals(timeIntervalsArray, time) {
Copy link

Choose a reason for hiding this comment

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

Название читаю, а понять что она делает не могу

for (var i = 0; i < timeIntervalsArray.length; i++) {
if (timeIntervalsArray[i].isTimeInInterval(time)) {
return true;

Choose a reason for hiding this comment

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

а зачем тут отступ перед return?)

}
}

return false;
}

function strDateToInt(strDate) {

Choose a reason for hiding this comment

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

Если честно, на первый взгляд тут адище происходит. Слайсы не подойдут?

var day = 0;
var strDateCopy = strDate.slice();
Copy link

Choose a reason for hiding this comment

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

Зачем это нужно?

if (DAYS_INDEXES[strDateCopy.slice(0, 2)] !== undefined) {
var strDayAndTime = strDateCopy.split(' ');
day = DAYS_INDEXES[strDayAndTime[0]];
strDateCopy = strDayAndTime[1];
}
var timeAndTimezone = strDateCopy.split('+');
var timezone = parseInt(timeAndTimezone[1], 10);
var strTime = timeAndTimezone[0];
var hoursAndMinutes = strTime.split(':');
var hours = parseInt(hoursAndMinutes[0], 10);
var minutes = parseInt(hoursAndMinutes[1], 10);

return day * MINUTES_IN_DAY + (hours - timezone) * MINUTES_IN_HOUR + minutes;
}

function getGangsterNames(schedule) {
var gangsterNames = [];
for (var gangsterName in schedule) {
if (schedule.hasOwnProperty(gangsterName)) {

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.

Ну если честно я не знаю как. https://learn.javascript.ru/object-for-in смотрел тут. hasOwnProperty для lint'a

Copy link

Choose a reason for hiding this comment

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

gangsterNames.push(gangsterName);
}
}

return gangsterNames;
}

function createTimeIntervalWithShift(time, shift) {
return new TimeInterval(strDateToInt(time.from) + shift, strDateToInt(time.to) + shift);
}

function simplifyTimesIntervals(timeIntervals, duration) {
Copy link

Choose a reason for hiding this comment

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

Название функции переводится как «Упростить временные интервалы». Я честно не понимаю что тут подразумевается

var newTimeIntervals = [];
timeIntervals.forEach(function (timeInterval) {
Copy link

Choose a reason for hiding this comment

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

Для фильтрации массивов придумали специальную функцию https://developer.mozilla.org/ru/docs/Web/JavaScript/Reference/Global_Objects/Array/filter

if (timeInterval.getLength() >= duration) {
newTimeIntervals.push(timeInterval);
}
});

return newTimeIntervals;
}

function addOrChangeLastTimeInterval(arrayIntervals, timeToStartNewInterval) {
Copy link

Choose a reason for hiding this comment

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

Из названия функции абсолютно не понятно что она делает. И по коду я тоже ничего не понял

if (arrayIntervals.length === 0 ||
arrayIntervals[arrayIntervals.length - 1].end !== timeToStartNewInterval - 1) {

Choose a reason for hiding this comment

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

В этой функции слишком часто используется arrayIntervals.length, лучше вынеси в отдельную переменную. Да и название покороче будет)

arrayIntervals.push(new TimeInterval(timeToStartNewInterval, timeToStartNewInterval));
} else {
arrayIntervals[arrayIntervals.length - 1].end += 1;
}
}

function getGoodTimeIntervals(schedule, gangsterNames) {
var goodTimesIntervals = [];
var isGoodTimeToHack = true;
var i = -1;
function isGangsterNotBusy(gangster) {
if (isTimeInIntervals(schedule[gangster], i)) {
isGoodTimeToHack = false;

Choose a reason for hiding this comment

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

можно сделать isGoodTimeToHack = !isTimeInIntervals(schedule[gangster], i)

Copy link
Author

Choose a reason for hiding this comment

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

Сделать, то можно. Только это не совсем правильно(я уже писал почему). Даже базовые тесты перестают проходить.

}
}

while (i < DAYS_TO_HACK * MINUTES_IN_DAY) {

Choose a reason for hiding this comment

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

В данном while у тебя i не изменяется, кроме как увеличивается при каждой итерации? Почему бы тогда не сделать это циклом?

Copy link
Author

Choose a reason for hiding this comment

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

функция isGangsterNotBusy использует внешние переменные, одной из которой является переменная i. А так как функции объявлять внутри цикла нельзя(lint), то следовательно нужно объявить эту переменную и функцию ее использующую заранее. Я вижу выход из этой ситуации созданием функции, которая возвращает другую функцию. Попробую это сделать.

Copy link
Author

Choose a reason for hiding this comment

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

исправил даже изящнее чем сам предполагал)

i++;
isGoodTimeToHack = true;
if (!isTimeInIntervals(schedule.Bank, i)) {
continue;
}
gangsterNames.forEach(isGangsterNotBusy);

Choose a reason for hiding this comment

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

А зачем ты это вынес в отдельную функцию? У тебя она вынесена далеко от этого места, да и содержит в себе 1 строчку...

Copy link
Author

Choose a reason for hiding this comment

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

В lint'е стоит ограничение, что нельзя создавать функции внутри цикла


if (isGoodTimeToHack) {
addOrChangeLastTimeInterval(goodTimesIntervals, i);
}
}

return goodTimesIntervals;
}

function createScheduleWithTimeIntervals(bankWorkingHours, schedule, gangsterNames) {

Choose a reason for hiding this comment

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

Эту функцию можно разбить на наполнение нового расписания гангстерами и банком. И объединить результат как раз в этой функции.

var newSchedule = {};
var bankShift = parseInt(bankWorkingHours.from.split('+')[1], 10) * MINUTES_IN_HOUR;
Copy link

@chipolinka chipolinka Oct 28, 2016

Choose a reason for hiding this comment

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

Очень сложная строка. И давай у тебя весь парсинг будет вынесен в одно место?


gangsterNames.forEach(function (gangsterName) {
newSchedule[gangsterName] = [];
schedule[gangsterName].forEach(function (time) {
var intervalWithShift = createTimeIntervalWithShift(time, bankShift);
intervalWithShift.start++;
intervalWithShift.end--;
newSchedule[gangsterName].push(intervalWithShift);
});
});
newSchedule.Bank = [];
newSchedule.Bank.push(createTimeIntervalWithShift(bankWorkingHours, bankShift));
for (var dayIndex = 0; dayIndex < DAYS_TO_HACK - 1; dayIndex++) {
newSchedule.Bank.push(newSchedule.Bank[newSchedule.Bank.length - 1].getNextDay());
}

return newSchedule;
}

function timeToString(time) {
return time < 10 ? '0' + time.toString() : time.toString();
}

exports.getAppropriateMoment = function (schedule, duration, workingHours) {
console.info(schedule, duration, workingHours);
var gangsterNames = getGangsterNames(schedule);
var newSchedule = createScheduleWithTimeIntervals(workingHours, schedule, gangsterNames);
var goodTimesIntervals = getGoodTimeIntervals(newSchedule, gangsterNames);
goodTimesIntervals = simplifyTimesIntervals(goodTimesIntervals, duration);

Choose a reason for hiding this comment

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

Лучше создай новую переменную с соответствующим названием. А то не ясно, чем результат этой функции отличается от предыдущей. А так -- мне нравится организация кода 👍


return {

/**
* Найдено ли время
* @returns {Boolean}
*/
exists: function () {
return false;
return goodTimesIntervals.length !== 0;
},

/**
* Возвращает отформатированную строку с часами для ограбления
* Например,
* "Начинаем в %HH:%MM (%DD)" -> "Начинаем в 14:59 (СР)"
* @param {String} template
* @returns {String}
*/
format: function (template) {
return template;
if (!this.exists()) {

return '';

Choose a reason for hiding this comment

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

Кажется, тут лишний перенос строки

}

var startInterval = goodTimesIntervals[0];
var day = parseInt(startInterval.start / (MINUTES_IN_DAY), 10);
var hour = parseInt(startInterval.start / MINUTES_IN_HOUR, 10) - day * HOURS_IN_DAY;
var minutes = startInterval.start % MINUTES_IN_HOUR;

return template.replace('%DD', DAYS_NAMES[day]).replace('%HH', timeToString(hour))
.replace('%MM', timeToString(minutes));
},

/**
* Попробовать найти часы для ограбления позже [*]
* @star
* @returns {Boolean}
*/
tryLater: function () {
var startInterval = goodTimesIntervals[0];
if (this.exists() && (goodTimesIntervals.length > 1 ||
startInterval.getLength() >= duration + MINUTES_TO_START_LATER)) {
startInterval.start += MINUTES_TO_START_LATER;
goodTimesIntervals = simplifyTimesIntervals(goodTimesIntervals, duration);

return true;
}

return false;
}
};
Expand Down