-
Notifications
You must be signed in to change notification settings - Fork 77
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
Филиппович Алексей #53
base: master
Are you sure you want to change the base?
Conversation
🍅 Не пройден линтинг или базовые тесты |
🍅 Не пройден линтинг или базовые тесты |
🍅 Пройдено тестов 14 из 16 |
🍅 Не пройден линтинг или базовые тесты |
🍅 Пройдено тестов 14 из 16 |
🍅 Не пройден линтинг или базовые тесты |
🍅 Пройдено тестов 14 из 16 |
🍅 Не пройден линтинг или базовые тесты |
🍅 Не пройден линтинг или базовые тесты |
🍅 Пройдено тестов 14 из 16 |
🍅 Пройдено тестов 14 из 16 |
🍅 Не пройден линтинг или базовые тесты |
🍏 Пройдено тестов 16 из 16 |
|
||
var BANK_TIMEZONE = 0; // временная зона банка | ||
var MINUTES_IN_DAY = 24 * 60; | ||
var DATE_PATTERN = /(([БВНПРСТЧ]+) )?(\d+):(\d+)([+-]\d+)/i; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
сам придумал?
Danny: [], | ||
Rusty: [], | ||
Linus: [], | ||
Gang: [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
плохо так привязываться к каждому грабителю(
Это сейчас их 3, а если 50? 100? и тд
Gang: [] | ||
}; | ||
|
||
function tryRobABank(schedule, duration, workingHours) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
а расшифруй название? что за A
??
robberyIntervals.Linus.splice(0); | ||
robberyIntervals.Danny = getFreeIntervals(schedule.Danny); | ||
robberyIntervals.Rusty = getFreeIntervals(schedule.Rusty); | ||
robberyIntervals.Linus = getFreeIntervals(schedule.Linus); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
опять-таки плохо привязываться так к именам грабителей
var timeZoneOffset = BANK_TIMEZONE - Number(dateArray[5]); | ||
minutes += dayToMinutes(dateArray[2]); | ||
minutes += (Number(dateArray[3]) + timeZoneOffset) * 60; | ||
minutes += Number(dateArray[4]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
плохо то, что код трудночитаем( Приходится прям вспоминать а как же регексп разобьет строку и что такое dateArray[4]
, dateArray[3]
и тд
minutes += (Number(dateArray[3]) + timeZoneOffset) * 60; | ||
minutes += Number(dateArray[4]); | ||
|
||
return Math.min(minutes, MINUTES_IN_DAY * 3 - 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"волшебные" 3 и 1? Вынеси в константу) У тебя потом может стать больше, чем три дня на ограбление, и тебе везде это надо будет править. А если будет константа, то только ее
robberyEvent.ready = true; | ||
robberyEvent.day = WEEKDAYS[Math.floor(robberyTime / MINUTES_IN_DAY)]; | ||
robberyTime -= WEEKDAYS.indexOf(robberyEvent.day) * MINUTES_IN_DAY; | ||
var minutes = robberyTime % 60; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
60 тоже надо в константу вынести
if (first.from < second.from) { | ||
return -1; | ||
} | ||
if (first.to > second.to) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
а почему не воспользоваться ИЛИ, а писать 4 if
?
freeIntervals.push(getInterval(from, busyIntervals[i].from)); | ||
} | ||
// Если последний интервал, когда рабитель занят, не приходится на СР 23:59 | ||
if (busyIntervals[busyIntervals.length - 1].to !== MINUTES_IN_DAY * 3 - 1) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
тоже самое с "волшебными" 3 и 1
привет! |
🍅 |
🍏 Пройдено тестов 16 из 16 |
🍏 Пройдено тестов 16 из 16 |
🍅 Не пройден линтинг или базовые тесты |
🍅 Пройдено тестов 10 из 16 |
🍅 Не пройден линтинг или базовые тесты |
🍅 Пройдено тестов 10 из 16 |
🍅 Пройдено тестов 10 из 16 |
🍅 Не пройден линтинг или базовые тесты |
🍅 Не пройден линтинг или базовые тесты |
🍅 Пройдено тестов 10 из 16 |
🍅 Не пройден линтинг или базовые тесты |
🍅 Пройдено тестов 10 из 16 |
🍅 Не пройден линтинг или базовые тесты |
🍏 Пройдено тестов 16 из 16 |
var MINUTES_IN_DAY = 24 * MINUTES_IN_HOUR; | ||
var DAYS_FOR_ROBBERY = 3; | ||
var MAX_MINUTES = DAYS_FOR_ROBBERY * MINUTES_IN_DAY - 1; // макс. 3 дня на ограбление и -1 минута | ||
var DATE_PATTERN = /(([БВНПРСТЧ]{2})\s)?(\d{2}):(\d{2})([+-]\d{1,2})/i; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
сам составил?
exports.isStar = true; | ||
exports.isStar = false; | ||
|
||
var BANK_TIMEZONE = 0; // временная зона банка |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
переменные нао определять максимально близко к тому месту, где они потом используются
// и времени работы банка | ||
robberyIntervals.Gang = getRobberyIntervals(robberyIntervals, bankSchedule); | ||
// Из готовых диапазонов получаем время, когда можно ограбить банк | ||
// с учётом продолжительности ограбления (если вообще можно) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
за комментарии от души спасибо 👍
|
||
function getBankSchedule(workingHours) { | ||
var schedule = []; | ||
for (var day = 0; day < 3; day++) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3 в константу
// Например, СР 15:00+3 -> 3900 (2880 прошло с ПН по СР 00:00 и 17 часов = 1020) | ||
function dateToMinutes(date) { | ||
var timeZoneOffset = BANK_TIMEZONE - Number(DATE_PATTERN.exec(date)[5]); | ||
var day = DATE_PATTERN.exec(date)[2]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
вынеси DATE_PATTERN.exec(date) в переменную, незачем делать это каждый раз
var busyIntervals = getBusyIntervals(schedule); | ||
var from = 0; | ||
for (var i = 0; i < busyIntervals.length; i++) { | ||
// Если грабитель занят с в ПН 00:00, то интервала с ПН 00:00 до ПН 00:00 нет |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
вообще непонятный комментарий. Он совсем запутывает. И на самом деле вот это сомнительное место
getFreeIntervals и getBusyIntervals. Подумай как оптимизировать это?
🍅 |
No description provided.