Skip to content
201 changes: 171 additions & 30 deletions robbery.js
Original file line number Diff line number Diff line change
@@ -1,49 +1,190 @@
'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 daysDict = { 'ПН': 0, 'ВТ': 1, 'СР': 2 };
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
Author

Choose a reason for hiding this comment

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

Да Это по сути константа. Исправлю

var daysArray = ['ПН', 'ВТ', 'СР'];
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
Author

Choose a reason for hiding this comment

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

Для читаемости кода. daysDict.keys()[i] хуже чем daysArray[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.

по-моему, это не повод у всех словарей выделять ключи в массив) в конце концов, можешь уж список-то заново не создавать

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

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.

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

var DAYS_TO_HACK = 3;


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

this.getLength = function () {
Copy link
Copy Markdown

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.timeIsInInterval = function (time) {
return this.start <= time && time <= this.end;
};
}

function timeIsInIntervals(timeIntervalsArray, time) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

лучше isTimeInIntervals

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 i = 0; i < timeIntervalsArray.length; i++) {
if (timeIntervalsArray[i].timeIsInInterval(time)) {
return true;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

}
}

return false;
}

function strToIntDate(strDate) {
var day = 0;
var strSplit = [];
if (daysDict[strDate.slice(0, 2)] !== undefined) {
strSplit = strDate.split(' ');
day = daysDict[strSplit[0]];
strDate = strSplit[1];
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

лучше отдельно заведи переменную с датой, которая изначально будет равна strDate, а потом изменяй её.
дальше в этой функции ты постоянно перезаписываешь переменные, из-за этого вообще теряется понимание, что происходит и на каком этапе какое значение лежит в переменной. лучше введи новые переменные с понятными названиями или сделай парсинг как-то иначе)

}
strSplit = strDate.split('+');
var timezone = parseInt(strSplit[1]);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

в parseInt лучше указывать систему счисления, почитай про это)

strDate = strSplit[0];
strSplit = strDate.split(':');
var hours = parseInt(strSplit[0]);
var minutes = parseInt(strSplit[1]);

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

exports.getAppropriateMoment = function (schedule, duration, workingHours) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ой, эта функция больше 100 строчек занимает. постарайся декомпозировать.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

вообще лучше сделать так, чтобы функции занимали не больше 25 строк. так будет более читаемо и понятно. и меньше шанс допустить ошибку)

console.info(schedule, duration, workingHours);
var newSchedule = {
Danny: [],
Rusty: [],
Linus: [],
Bank: []
};

var bankTimezone = parseInt(workingHours.from.split('+')[1]);

var gangsterNames = ['Danny', 'Rusty', 'Linus'];
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

опять дублирование ключей словаря. а что ты будешь делать, если компаньоны соберутся поменять состав?


gangsterNames.forEach(function (gangsterName) {
schedule[gangsterName].forEach(function (time) {
newSchedule[gangsterName].push(new TimeInterval(
strToIntDate(time.from) + 1 + bankTimezone * MINUTES_IN_HOUR,
strToIntDate(time.to) - 1 + bankTimezone * MINUTES_IN_HOUR));
});
});
newSchedule.Bank.push(new TimeInterval(
strToIntDate(workingHours.from) + bankTimezone * MINUTES_IN_HOUR,
strToIntDate(workingHours.to) + bankTimezone * MINUTES_IN_HOUR));
Copy link
Copy Markdown

@chipolinka chipolinka Oct 24, 2016

Choose a reason for hiding this comment

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

меня смущает, что у тебя 4 раза встречается строка
strToIntDate(something) + bankTimezone * MINUTES_IN_HOUR
это никак не сделать поизящнее?
имею в виду DRY

for (var dayIndex = 0; dayIndex < DAYS_TO_HACK - 1; dayIndex++) {
newSchedule.Bank.push(newSchedule.Bank[newSchedule.Bank.length - 1].getNextDay());
}

function getGoodTimeIntervals() {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

какая-то больно сложная функция получилась. не лучше ли вынести из неё мелкие функции наружу?

var goodTimesInt = [];

function createGoodTimesIntArray() {

var goodTimeToHack = true;
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.

а ещё название этой переменной предполагает, что в ней хранится время.

var i = 0;
function gangsterIsNotBusy(gangster) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

название функции следует начинать с глагола

if (timeIsInIntervals(newSchedule[gangster], i)) {
goodTimeToHack = false;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

goodTimeToHack = !timeInIntervals(newSchedule[gangster], i) ?

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.

В случае если время неподходит первому, но подходит второму и третьему, то значение переменной будет true. Что не совсем то что нужно.

}
}

for (; i < DAYS_TO_HACK * MINUTES_IN_DAY; 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.

тебе не кажется, что здесь больше подойдет while?

goodTimeToHack = true;

if (!timeIsInIntervals(newSchedule.Bank, i)) {
continue;
}

gangsterNames.forEach(gangsterIsNotBusy);

if (goodTimeToHack) {
goodTimesInt.push(i);
}
}
}

createGoodTimesIntArray();

var goodTimesIntervals = [];

for (var i = 0; i < goodTimesInt.length; i++) {
if (i === 0 || goodTimesInt[i - 1] + 1 !== goodTimesInt[i]) {
goodTimesIntervals.push(new TimeInterval(goodTimesInt[i], goodTimesInt[i]));
} else {
goodTimesIntervals[goodTimesIntervals.length - 1].end += 1;
}
}

return goodTimesIntervals;
}

var goodTimesIntervals = getGoodTimeIntervals();

function simplifyTimesIntervals() {
var newTimeIntervals = [];
goodTimesIntervals.forEach(function (timeInterval) {
if (timeInterval.getLength() >= duration) {
newTimeIntervals.push(timeInterval);
}
});
goodTimesIntervals = newTimeIntervals;
}

simplifyTimesIntervals();

return {

/**
* Найдено ли время
* @returns {Boolean}
*/
exists: function () {
for (var i = 0; i < goodTimesIntervals.length; i++) {
if (goodTimesIntervals[i].getLength() >= duration) {

return true;
}
}

return false;
},

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

return '';
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 timeToString(time) {
if (time < 10) {

return '0' + time.toString();
}

return time.toString();
}
var startInterval = goodTimesIntervals[0];
var day = parseInt(startInterval.start / (MINUTES_IN_DAY));
var hour = parseInt(startInterval.start / MINUTES_IN_HOUR) - day * HOURS_IN_DAY;
var minutes = startInterval.start % MINUTES_IN_HOUR;

return template.replace('%DD', daysArray[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;
simplifyTimesIntervals();

return true;
}

return false;
}
};
Expand Down