Skip to content
191 changes: 161 additions & 30 deletions robbery.js
Original file line number Diff line number Diff line change
@@ -1,49 +1,180 @@
'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.

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


function TimeInterval(start, end) {
this.start = start;
this.end = end;
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.

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


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 + 24 * 60, this.end + 24 * 60);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

числа нужно вынести в константы

};

this.timeInInterval = function (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.

в названии функции надо указывать, что она делает (то бишь глагол)

return this.start <= time && time <= this.end;
};
}

function timeInIntervals(timeIntervalsArray, time) {
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.

аналогично с timeInInterval

for (var i = 0; i < timeIntervalsArray.length; i++) {
if (timeIntervalsArray[i].timeInInterval(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(str) {
var day = 0;
if (daysDict[str.slice(0, 2)] !== undefined) {
day = daysDict[str.split(' ')[0]];
str = str.split(' ')[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.

str.split(' ') наверное стоит вынести в переменную, раз часто используется

}
var timezone = parseInt(str.split('+')[1]);
str = str.split('+')[0];
var h = parseInt(str.split(':')[0]);
var m = parseInt(str.split(':')[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.

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


return day * 24 * 60 + (h - timezone) * 60 + m;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

константы

}

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 * 60, strToIntDate(time.to) - 1 + bankTimezone * 60));
});
});
newSchedule.Bank.push(new TimeInterval(strToIntDate(workingHours.from) +
bankTimezone * 60, strToIntDate(workingHours.to) + bankTimezone * 60));
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

кажется, тут есть дублирование... и константы, конечно.

newSchedule.Bank.push(newSchedule.Bank[0].getNextDay());
newSchedule.Bank.push(newSchedule.Bank[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 gangsterNotBusy(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 (timeInIntervals(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 < 3 * 24 * 60; 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.

ой, как много констант

goodTimeToHack = true;

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

gangsterNames.forEach(gangsterNotBusy);

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

Потому что я могу)

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 / (24 * 60));
var hour = parseInt(startInterval.start / 60) - day * 24;
var minutes = startInterval.start % 60;

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

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

кажется, что это можно переписать в один if, ну или как-то более сокращенно

startInterval.start += 30;
simplifyTimesIntervals();

return true;
}

return false;
}

return false;
}
};
Expand Down