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

Трофимов Павел #43

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Changes from 2 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
193 changes: 186 additions & 7 deletions robbery.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,163 @@
'use strict';

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

Choose a reason for hiding this comment

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

Зачем ты удалил все JsDoc? Давай их вернем обратно. Хорошей практикой считается документировать публичные методы, так как по параметрам сложно догадаться, что они содержат


var ROBBERY_DAYS = ['ПН', 'ВТ', 'СР'];
var MS_IN_MINUTE = 60 * 1000;
// сдвиг на 30 минут
var TIME_SHIFT = 30;

function getBankSchedule(bankHours, bankTimezone) {

return ROBBERY_DAYS.map(function (day) {
var from = day + ' ' + bankHours.from;
var to = day + ' ' + bankHours.to;

return getNewInterval(from, to, bankTimezone);
});
}

function getNewDate(date, bankTimezone) {
var timeRegExp = /([А-Я]{2}) (\d{2}):(\d{2})\+(\d+)/g;
var parsedTime = timeRegExp.exec(date);
var day = ROBBERY_DAYS.indexOf(parsedTime[1]) + 1;
Copy link

Choose a reason for hiding this comment

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

ROBBERY_DAYS.indexOf(parsedTime[1]) + 1
В условии сказано, что проверять данные не надо, но допустим в расписании появится ЧТ. ЧТ, как день, является валидным значением, но твой код вернет 0(воскресенье), вместо 4(четверга).

Получается, что шаг в лево и твое решение перестанет работать. Надо переписать на что-то более надежное

var hour = parsedTime[2] - (parseInt(parsedTime[4], 10) - bankTimezone);

return new Date(2016, 7, day, hour, parsedTime[3]);
}

function parseSchedule(schedule, bankTimezone) {
var parsedSchedule = [];
Object.keys(schedule).forEach(function (robberName) {
schedule[robberName].forEach(function (interval) {
var from = interval.from;
var to = interval.to;
parsedSchedule.push(getNewInterval(from, to, bankTimezone));
});
});

return parsedSchedule;
}


function getNewInterval(from, to, timezone) {

return {
from: getNewDate(from, timezone),
to: getNewDate(to, timezone)
};
}


function getSortedIntervals(schedule, bankTimezone) {
var sortedSchedule = parseSchedule(schedule, bankTimezone);
Copy link

Choose a reason for hiding this comment

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

Тут еще пока не отсортировано. Отсортированным становится только на следующем шаге

sortedSchedule.sort(function (a, b) {
return a.from - b.from;
});
var joinedSchedule = joinSchedule(sortedSchedule);

return getFreeSchedule(joinedSchedule, bankTimezone);
}

function checkInterval(time, interval) {

Choose a reason for hiding this comment

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

В названии не отражено, на что именно ты проверяешь данный интервал. Раз ты возвращаешь булевское значение, то может быть лучше назвать как-нибудь isIntersected или isIncluded или что-то в этом роде (посмотри, что больше подходит)


return interval.from <= time && time <= interval.to;
}

function createDate(currentTime, firstTime) {

Choose a reason for hiding this comment

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

еще бы вот тут переименовать, ты же выбираешь максимум из дат, как я поняла; то есть по сути позднее из двух времен. И раз ты возвращаешь дату, то можно назвать getLaterTime. В аргументы передавай firstTime и secondTime

Choose a reason for hiding this comment

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

Ну или getLaterDate

var greater = Math.max(currentTime, firstTime);

return new Date(greater);
}


function joinSchedule(schedule) {
var busyIntervals = [];
var totalDayInterval = schedule[0];

Choose a reason for hiding this comment

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

Не поняла, что за total day interval, подумай над другим названием


for (var i = 0; i < schedule.length; i++) {
var check = checkInterval(schedule[i].from, totalDayInterval);

Choose a reason for hiding this comment

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

эту переменную тоже лучше переименовать, чтобы было понятно, что в ней лежит либо true, либо false


if (check) {
totalDayInterval.to = createDate(schedule[i].to, totalDayInterval.to);
} else {
busyIntervals.push(totalDayInterval);
totalDayInterval = schedule[i];
}
}
Copy link

Choose a reason for hiding this comment

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

Почему тут for, а везде выше forEach?

busyIntervals.push(totalDayInterval);

return busyIntervals;
}


function isCrossedIntervals(bankInterval, freeInterval) {
var checkTimeFrom = checkInterval(bankInterval.from, freeInterval);
var checkTimeTo = checkInterval(bankInterval.to, freeInterval);

Choose a reason for hiding this comment

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

checkTimeFrom и checkTimeTo надо бы переименовать, а то непонятно, что за проверки времени и что в них содержится булевское значение

var compareTimes = freeInterval.from > bankInterval.from &&

Choose a reason for hiding this comment

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

Если я правильно поняла, то compareTimes - это isRobberyTime

freeInterval.to < bankInterval.to;

return checkTimeFrom || checkTimeTo || compareTimes;
}

function getBankTimezone(time) {
var bankTimezone = /\+(\d+)/.exec(time)[1];

return parseInt(bankTimezone, 10);
}
Copy link

Choose a reason for hiding this comment

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

Ты определяешь часовой пояс банка, а потом везде его прокидываешь в качестве аргумента. Проще сохранить в переменную


function getMomentsForRobbery(freeSchedule, bankSchedule, duration) {
var timesToRob = [];

Choose a reason for hiding this comment

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

Лучше не сокращать слова в названиях: timesOfRobbery либо robberyTimes


bankSchedule.forEach(function (bankInterval) {
freeSchedule.forEach(function (freeInterval) {
if (isCrossedIntervals(bankInterval, freeInterval)) {
var crossInterval = {
from: new Date(Math.max(bankInterval.from, freeInterval.from)),
to: new Date(Math.min(bankInterval.to, freeInterval.to))
};

var laterTime = new Date(crossInterval.from.getTime() + (duration * MS_IN_MINUTE));
if (checkInterval(laterTime, crossInterval)) {
timesToRob.push(crossInterval);
}
}
});
});
Copy link

Choose a reason for hiding this comment

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

Тут все интервалы сравниваются с другими интервалами, но это не очень хорошо с точки зрения производительности. Зачем сравнивать время работы банка в понедельник с свободным временем грабителей во вторник или среду. Попробуй как-нибудь оптимизировать проверку по дням


return timesToRob;
}

function getFreeSchedule(busyTime, bankTimezone) {
var freeIntervals = [];
var currentFreeInterval = {};

currentFreeInterval.from = getNewDate('ПН 00:00+' + bankTimezone, bankTimezone);
Copy link

Choose a reason for hiding this comment

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

'ПН 00:00+' - надо унести в константы.

busyTime.forEach(function (currentBusyInterval) {
if (currentBusyInterval.from === currentBusyInterval.to) {
return;
}
Copy link

Choose a reason for hiding this comment

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

А в каком случае может отработать это условие? Ведь если у интервала начало и конец совпадают, то это уже не интервал, а момент времени.

Copy link

Choose a reason for hiding this comment

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

🍅

currentFreeInterval.to = currentBusyInterval.from;
freeIntervals.push(currentFreeInterval);
currentFreeInterval = {};
currentFreeInterval.from = currentBusyInterval.to;
});
currentFreeInterval.to = getNewDate('СР 23:59+' + bankTimezone, bankTimezone);
freeIntervals.push(currentFreeInterval);

return freeIntervals;
}

function formatTime(time) {

return (time < 10 ? '0' : '') + time;
}

function createLaterTime(date, shift) {

return new Date(date.getTime() + (shift * MS_IN_MINUTE));
}

/**
* @param {Object} schedule – Расписание Банды
* @param {Number} duration - Время на ограбление в минутах
Expand All @@ -15,7 +167,10 @@ exports.isStar = true;
* @returns {Object}
*/
exports.getAppropriateMoment = function (schedule, duration, workingHours) {
console.info(schedule, duration, workingHours);
var bankTimezone = getBankTimezone(workingHours.from);
var freeTimeIntervals = getSortedIntervals(schedule, bankTimezone);
var bankSchedule = getBankSchedule(workingHours, bankTimezone);
var robberyTimes = getMomentsForRobbery(freeTimeIntervals, bankSchedule, duration);

return {

Expand All @@ -24,7 +179,7 @@ exports.getAppropriateMoment = function (schedule, duration, workingHours) {
* @returns {Boolean}
*/
exists: function () {
return false;
return robberyTimes.length > 0;
},

/**
Expand All @@ -35,7 +190,16 @@ exports.getAppropriateMoment = function (schedule, duration, workingHours) {
* @returns {String}
*/
format: function (template) {
return template;
if (!this.exists()) {
return '';
}

var time = robberyTimes[0].from;

return template
.replace('%HH', formatTime(time.getHours()))
.replace('%DD', ROBBERY_DAYS[time.getDay() - 1])
.replace('%MM', formatTime(time.getMinutes()));
},

/**
Expand All @@ -44,6 +208,21 @@ exports.getAppropriateMoment = function (schedule, duration, workingHours) {
* @returns {Boolean}
*/
tryLater: function () {
if (this.exists()) {
var laterTime = createLaterTime(robberyTimes[0].from, TIME_SHIFT);
var newEndTime = createLaterTime(laterTime, duration);
if (checkInterval(newEndTime, robberyTimes[0])) {
robberyTimes[0].from = laterTime;

return true;
// из тестов "не должен сдвигать момент, если более позднего нет"
} else if (robberyTimes.length > 1) {
robberyTimes.shift();

return true;
}
}

return false;
}
};
Expand Down