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

Лебедев Никита #55

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
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
16 changes: 8 additions & 8 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

314 changes: 306 additions & 8 deletions robbery.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,266 @@
'use strict';

exports.isStar = true;

const CURRENT_YEAR = 2017;

Choose a reason for hiding this comment

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

Подозрительные константы. Зачем именно текущий год и месяц? Либо брать всегда текущий, не забивая его в константы. Либо взять какие-то дефолтные значения. Сейчас выглядит так, что эти значение через пару недель протухнут, и надо будет переписывать программу

const CURRENT_MONTH = 10;
const HOUR_IN_MILLISEC = 1000 * 60 * 60;
const MINUTE_IN_MILLISEC = 1000 * 60;
const HALF_HOUR = 30;

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.

И я бы переименовала эту константу. Так как полчаса - на самом деле тоже параметр задачи, может поменяться и на 40 минут, и на 10.

const HALF_HOUR_IN_MILLISEC = HALF_HOUR * MINUTE_IN_MILLISEC;
const ROBBERY_DAYS = ['ПН', 'ВТ', 'СР'];


/**
* Сделано задание на звездочку
* Реализовано оба метода и tryLater
* @param {String} string
* @returns {String}
*/
exports.isStar = true;
function getDay(string) {
return string.substr(0, 2);
}

/**
* @param {String} day
* @returns {Number}
*/
function getNumberOfDay(day) {
return ROBBERY_DAYS.indexOf(getDay(day)) + 1;

Choose a reason for hiding this comment

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

эта функция точно всегда будет правильно работать? Дней 7, а в ROBBERY_DAYS их только 3. При этом в расписании может попасться и четверг, и пятница, и тд

}

/**
* @param {String} string
* @returns {Number}
*/
function getHours(string) {
return string.substring(3, 5);
}

/**
* @param {String} string
* @returns {Number}
*/
function getMinutes(string) {
return string.substring(6, 8);
}


/**
* @param {String} time
* @returns {Number}
*/
function getTimeZone(time) {
return time.substr(-1);

Choose a reason for hiding this comment

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

Зона может быть и двузначным числом

}

/**
* @param {String} string
* @param {Number} day
* @param {Number} hours
* @param {Number} minutes
* @returns {String}
*/
function getCorrectFormat(string, day, hours, minutes) {

Choose a reason for hiding this comment

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

У тебя много функций, связанных с работой с датами и временем. Можно было бы вынести их в отдельный файл-библиотеку

return string
.replace('%DD', day)
.replace('%HH', hours)
.replace('%MM', minutes);
}

/**
* @param {Array} schedule
* @returns {Array}
*/
function crossElements(schedule) {
let connectedSchedule = [];
let from = 0;
let to = 0;
schedule.forEach((time, i) => {
from = time.from;
to = time.to;
schedule.forEach((time2, j) => {
if (i !== j && from <= time2.to && to >= time2.from) {
from = Math.max(from, time2.from);
to = Math.min(to, time2.to);
}
});
connectedSchedule.push({ from, to });
});

return connectedSchedule;
}

/**
* @param {String} time
* @param {Number} day
* @param {Number} timezone
* @returns {Object}
*/
function bankUTCTime(time, day, timezone) {
return Date.UTC(
CURRENT_YEAR,
CURRENT_MONTH,
day + 1,
time.substring(0, 2),

Choose a reason for hiding this comment

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

У тебя же есть функции getDay и getHours, почему здесь ими не пользуешься?

time.substring(3, 5)) - timezone;
}

/**
* @param {Object} time
* @returns {Array}
*/
function toBankUTCTime(time) {

Choose a reason for hiding this comment

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

Название функции не соответствует происходящему. Кажется, что просто переводится время. Но на самом деле, возвращается расписание банка

let scheduleBank = [];
let timezone = getTimeZone(time.from) * HOUR_IN_MILLISEC;
for (let i = 0; i <= 2; i++) {

Choose a reason for hiding this comment

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

Почему захардкожено число 2? Нужно общее решение. Например, на ограбление может быть не 3 дня, а 5. Нужно убрать хардкод количества дней

let from = bankUTCTime(time.from, i, timezone);
let to = bankUTCTime(time.to, i, timezone);
scheduleBank.push({ from, to });
}

return scheduleBank;
}

/**
* @param {Object} time
* @param {Number} timezone
* @returns {Object}
*/
function utcTimeForRobber(time, timezone) {

Choose a reason for hiding this comment

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

Эта функция очень похожа на bankUTCTime. Лучше избегать копипасты, поэтому попробуй их объединить

return Date.UTC(
CURRENT_YEAR,
CURRENT_MONTH,
getNumberOfDay(time),
getHours(time),
getMinutes(time)) - timezone;
}

/**
* @param {Object} time
* @returns {Object}
*/
function toUTC(time) {
let timezone = getTimeZone(time.from) * HOUR_IN_MILLISEC;
time.from = utcTimeForRobber(time.from, timezone);
time.to = utcTimeForRobber(time.to, timezone);

return time;
}

/**
* @param {Object} schedule
* @returns {Array}
*/
function mergeTime(schedule) {
let merged = [];
Object.keys(schedule).forEach(robber => {
schedule[robber].forEach(time => {
merged.push({ from: time.from, to: time.to });
});
});

return merged;
}

/**
* @param {Array} schedule
* @returns {Array}
*/
function mixSchedule(schedule) {

Choose a reason for hiding this comment

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

Название странное. Я же правильно понимаю, что тут ты слепляешь входящие друг в друга интервалы? По названию этого не понять

let connectedSchedule = [];
let from = 0;
let to = 0;
schedule.forEach((time, i) => {
from = time.from;
to = time.to;
schedule.forEach((time2, j) => {
if (i !== j &&
from <= time2.to && to >= time2.from) {
from = Math.min(from, time2.from);
to = Math.max(to, time2.to);
}
});
connectedSchedule.push({ from, to });
});

return connectedSchedule;
}

/**
* @param {Array} schedule
* @returns {Array}
*/
function deleteCopies(schedule) {
if (schedule.length !== 0) {
schedule.reduceRight((previousValue, currentValue, index) => {
if (currentValue.from === previousValue.from && currentValue.to === previousValue.to) {
schedule.splice(index, 1);

Choose a reason for hiding this comment

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

Я бы не рекомендовала делать splice на массиве, по которому проходит цикл. Очень легко ошибиться и сломать что-нибудь. Лучше создавай новый массив, тогда и reduceRight можно будет заменить на что-нибудь другое

}

return currentValue;
});
}

return schedule;
}

/**
* @param {Array} commonSchedule
* @param {Array} bankSchedule
* @param {Number} timezone
* @param {Number} duration
* @returns {Array}
*/
function reverseSchedule(commonSchedule, bankSchedule, timezone, duration) {

Choose a reason for hiding this comment

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

И опять же, можно переименовать функцию. Так как по факту, она возвращает доступное для ограбления время

if ((bankSchedule[0].to - bankSchedule[0].from) - duration * MINUTE_IN_MILLISEC < 0) {
return [];
}
let freeTimes = [];
let range = {
from: Date.UTC(CURRENT_YEAR, CURRENT_MONTH, 1, 0, 0) - timezone,
to: Date.UTC(CURRENT_YEAR, CURRENT_MONTH, 3, 23, 59) - timezone };

Choose a reason for hiding this comment

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

3 опять похоже на захардкоженную константу

bankSchedule.forEach(day => {

Choose a reason for hiding this comment

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

Этот шаг не относится к reverseSchedule, лучше вынести его в отдельную функцию

commonSchedule.push({ from: range.from, to: day.from });
commonSchedule.push({ from: day.to, to: range.to });
});
commonSchedule.forEach(badTime => {
if (Math.max(range.from, badTime.from) === Math.min(range.to, badTime.from) &&
badTime.from !== range.from) {
freeTimes.push({ from: range.from, to: badTime.from });
}
if (Math.max(range.from, badTime.to) === Math.min(range.to, badTime.to) &&
badTime.to !== range.to) {
freeTimes.push({ from: badTime.to, to: range.to });
}
});

return freeTimes;
}

/**
* @param {Array} freeTime
* @param {Number} duration
* @returns {Array}
*/
function findFreeTimeForRobbers(freeTime, duration) {
duration *= MINUTE_IN_MILLISEC;
let robberyTimes = [];
freeTime.forEach(time => {
if (time.to - time.from >= duration) {
robberyTimes.push(time);
}
});

return robberyTimes;
}

/**
* @param {Object} element1
* @param {Object} element2
* @returns {Object}
*/
function forSort(element1, element2) {

Choose a reason for hiding this comment

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

Тут тоже название ничего не говорит о том, что и как сортируешь

return element1.from - element2.from;
}

/**
* @param {Object} schedule – Расписание Банды
Expand All @@ -14,17 +270,33 @@ exports.isStar = true;
* @param {String} workingHours.to – Время закрытия, например, "18:00+5"
* @returns {Object}
*/
exports.getAppropriateMoment = function (schedule, duration, workingHours) {
console.info(schedule, duration, workingHours);

exports.getAppropriateMoment = (schedule, duration, workingHours) => {
let timezone = getTimeZone(workingHours.from);
let bankSchedule = toBankUTCTime(workingHours);
let mergedTime = mergeTime(schedule).map(time => toUTC(time));
let commonTime = mixSchedule(mergedTime);
commonTime.sort(forSort);
let reversedSchedule = reverseSchedule(
deleteCopies(commonTime), bankSchedule, timezone * HOUR_IN_MILLISEC, duration);
let freeTime = crossElements(reversedSchedule);
freeTime.sort(forSort);
let timeForRobbery = findFreeTimeForRobbers(deleteCopies(freeTime), duration);
let start = 0;
if (timeForRobbery.length > 0) {
start = timeForRobbery[0].from;
}

return {

interval: 0,

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

/**
Expand All @@ -35,7 +307,16 @@ exports.getAppropriateMoment = function (schedule, duration, workingHours) {
* @returns {String}
*/
format: function (template) {
return template;
if (!this.exists()) {
return '';
}
let time = new Date(start + timezone * HOUR_IN_MILLISEC);

return getCorrectFormat(
template,
ROBBERY_DAYS[time.getUTCDate() - 1],
('0' + time.getUTCHours()).slice(-2),
('0' + time.getUTCMinutes()).slice(-2));
},

/**
Expand All @@ -44,6 +325,23 @@ exports.getAppropriateMoment = function (schedule, duration, workingHours) {
* @returns {Boolean}
*/
tryLater: function () {
if (!this.exists()) {
return false;
}
if (this.interval < timeForRobbery.length) {
if (start + HALF_HOUR_IN_MILLISEC + duration * MINUTE_IN_MILLISEC <=
timeForRobbery[this.interval].to) {
start += HALF_HOUR_IN_MILLISEC;

return true;
} else if (this.interval < timeForRobbery.length - 1) {
this.interval++;
start = timeForRobbery[this.interval].from;

return true;
}
}

return false;
}
};
Expand Down