Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
.idea
/node_modules
*.log
index.1.js
249 changes: 245 additions & 4 deletions robbery.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,211 @@
*/
exports.isStar = true;

function parseTime(timeString, containsDay) {
let daysOfWeek = ['ПН', 'ВТ', 'СР', 'ЧТ', 'ПТ', 'СБ', 'ВС'];

Choose a reason for hiding this comment

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

Везде, где присвоение происходит один раз, имеет смысл использовать const вместо let.

let day = 0;
if (containsDay) {
let dayName = timeString.match(/(..)/)[1];

Choose a reason for hiding this comment

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

В данном случае регулярное выражение излишне. Лучше substr.

day = daysOfWeek.indexOf(dayName);
}
let numbers = timeString.match(/(\d+):(\d+)\+(\d+)$/);
let hours = Number.parseInt(numbers[1]);

Choose a reason for hiding this comment

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

Для таких целей хватит просто Number.

let minutes = Number.parseInt(numbers[2]);
let timeZone = Number.parseInt(numbers[3]);

return (24 * day + hours - timeZone) * 60 + minutes;
}

function convertRobbersSchedule(schedule) {
let newSchedule = [];

Choose a reason for hiding this comment

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

Через reduce получится короче и выразительнее.

let keys = ['Danny', 'Rusty', 'Linus'];
keys.forEach(key => {
schedule[key].forEach(interval => {
newSchedule.push({
from: parseTime(interval.from, true),
to: parseTime(interval.to, true)
});
});
});

return newSchedule;
}

function removeEmptyIntervals(schedule) {
return schedule.filter(interval => interval.from !== interval.to);
}

function checkIntersection(a, b) {
return ((a.from <= b.to && a.from >= b.from) ||
(a.to <= b.to && a.to >= b.from) ||
(a.from < b.from && a.to > b.to));
}

function uniteIntersectedIntervals(a, b) {
return {
from: Math.min(a.from, b.from),
to: Math.max(a.to, b.to)
};
}

function addInterval(intervals, newInterval) {
let result = [];
intervals.forEach(interval => {

Choose a reason for hiding this comment

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

Тоже сделать через reduce.

if (checkIntersection(interval, newInterval)) {
newInterval = uniteIntersectedIntervals(interval, newInterval);
} else {
result.push(interval);
}
});
result.push(newInterval);

return result;
}

function removeScheduleIntersections(schedule) {
let unitedSchedule = [];

Choose a reason for hiding this comment

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

И тут тоже редьюсер напрашивается. )

schedule.forEach(interval => {
unitedSchedule = addInterval(unitedSchedule, interval);
});

return unitedSchedule;
}

function sortSchedule(schedule) {
return schedule.sort((a, b) => {
a = a.from;

Choose a reason for hiding this comment

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

Почему именно в такой форме?
Все-таки оба from-а лучше вынести в отдельные переменные, чем переприсваивать аргументы.

b = b.from;
if (a < b) {
return -1;
} else if (a > b) {
return 1;
}

Choose a reason for hiding this comment

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

Лишняя пустая строка.

return 0;
});
}

function convertBankSchedule(schedule) {
let from = parseTime(schedule.from, false);
let to = parseTime(schedule.to, false);

return [0, 1, 2].map(day => {
return {
from: from + day * 24 * 60,
to: to + day * 24 * 60
};
});
}

function joinOnTimeline(schedule, bankSchedule) {
let timeline = [];
schedule.forEach(busyInterval => {
timeline.push({
time: busyInterval.from,
type: 'busyStart'
});
timeline.push({
time: busyInterval.to,
type: 'busyEnd'
});
});
bankSchedule.forEach(bankInterval => {
timeline.push({
time: bankInterval.from,
type: 'bankStart'
});
timeline.push({
time: bankInterval.to,
type: 'bankEnd'
});
});
timeline.sort((a, b) => {
if (a.time < b.time) {
return -1;
} else if (a.time > b.time) {
return 1;
}
let sortPriority = ['busyEnd', 'bankStart', 'busyStart', 'bankEnd'];

return sortPriority.indexOf(a.type) - sortPriority.indexOf(b.type);
});

return timeline;
}

function switchState(previous, type) {
switch (type) {
case 'bankStart':
previous.bank = true;
break;
case 'bankEnd':
previous.bank = false;
break;
case 'busyStart':
previous.busy = true;
break;
case 'busyEnd':
previous.busy = false;
break;
default:
break;
}
}

function findRobberyIntervals(schedule, bankSchedule) {
let freeTime = [];
let timeline = joinOnTimeline(schedule, bankSchedule);
let addFreeTime = (from, to) => freeTime.push(
{
from: from, to: to

Choose a reason for hiding this comment

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

Использовать object property shorthand: { from, to }

}
);
timeline.reduce((previous, point) => {
if (previous === null) {
previous = {
start: null,
busy: false,
bank: false
};
}
switchState(previous, point.type);
if (previous.start !== null) {
addFreeTime(previous.start, point.time);
previous.start = null;
}
if (!previous.busy && previous.bank) {
previous.start = point.time;
}

return previous;
}, null);

return freeTime;
}

function findStartTimes(robberyIntervals, duration) {

Choose a reason for hiding this comment

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

Этот метод тоже, по сути, редьюсер.

let startTimes = [];
let lastRobberyTime = 0;

Choose a reason for hiding this comment

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

(Опционально)
Строки 193-196 можно загнать в форматированный тернарный оператор.

if (robberyIntervals.length > 0) {
lastRobberyTime = robberyIntervals[0].from - 30;
}
robberyIntervals.forEach(interval => {
for (let time = Math.max(interval.from, lastRobberyTime + 30);

Choose a reason for hiding this comment

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

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

// for (let time = interval.from;

Choose a reason for hiding this comment

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

Убрать комментарий.

time + duration <= interval.to;
time += 30) {
lastRobberyTime = time;
startTimes.push(time);
}
});

return startTimes;
}

function extractBankTimeZone(time) {
return Number.parseInt(time.match(/\+(\d+)$/)[1]);
Copy link

@YuuKohaku YuuKohaku Oct 23, 2017

Choose a reason for hiding this comment

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

Можно было обойтись Number();
Number.parseInt ведет себя аналогично parseInt, а здесь возможность конверсии между системами счисления вряд ли понадобится.
Еще: таймзона может быть и с минусом.

}

/**
* @param {Object} schedule – Расписание Банды
* @param {Number} duration - Время на ограбление в минутах
Expand All @@ -15,16 +220,26 @@ exports.isStar = true;
* @returns {Object}
*/
exports.getAppropriateMoment = function (schedule, duration, workingHours) {
console.info(schedule, duration, workingHours);
schedule = convertRobbersSchedule(schedule);

Choose a reason for hiding this comment

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

Создать отдельную переменную в let для переприсвоений или обойтись без переприсвоений. Присваивать значение аргументам функции не стоит.

schedule = removeEmptyIntervals(schedule);
schedule = removeScheduleIntersections(schedule);
schedule = sortSchedule(schedule);
let bankSchedule = convertBankSchedule(workingHours);
let robberyIntervals = findRobberyIntervals(schedule, bankSchedule);
let startTimes = findStartTimes(robberyIntervals, duration);
let bankTimeZone = extractBankTimeZone(workingHours.from);

return {
startTimes: startTimes,
currentPos: 0,
bankTimeZone: bankTimeZone,

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

/**
Expand All @@ -35,7 +250,28 @@ exports.getAppropriateMoment = function (schedule, duration, workingHours) {
* @returns {String}
*/
format: function (template) {
return template;
if (this.startTimes.length === 0) {
return '';
}
let daysOfWeek = ['ПН', 'ВТ', 'СР', 'ЧТ', 'ПТ', 'СБ', 'ВС'];

Choose a reason for hiding this comment

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

(Опционально)
А вот это можно и в константу наверх вынести.

let time = this.startTimes[this.currentPos] + this.bankTimeZone * 60;
let day = daysOfWeek[Math.floor(time / (24 * 60))];
time = time % (24 * 60);

Choose a reason for hiding this comment

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

Давай в отдельную переменную. Трудно следить за ходом мысли, когда содержимое переменной постоянно меняет смысл.

let formatNumber = number => {
let text = number.toString();
if (text.length === 1) {
text = '0'.concat(text);
}

return text;
};
let hours = formatNumber(Math.floor(time / 60));
let minutes = formatNumber(time % 60);

return template
.replace(/%DD/g, day)
.replace(/%HH/g, hours)
.replace(/%MM/g, minutes);
},

/**
Expand All @@ -44,7 +280,12 @@ exports.getAppropriateMoment = function (schedule, duration, workingHours) {
* @returns {Boolean}
*/
tryLater: function () {
return false;
if (this.currentPos === this.startTimes.length - 1 || this.startTimes.length === 0) {

Choose a reason for hiding this comment

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

Если this.startTimes.length вынести в отдельную переменную, условие получится короче, а смысл - понятнее. Кроме того, здесь можно использовать this.exists(), опционально.

return false;
}
this.currentPos++;

return true;
}
};
};