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

Филиппович Алексей #53

Open
wants to merge 35 commits into
base: master
Choose a base branch
from
Open

Conversation

feniwe
Copy link

@feniwe feniwe commented Oct 26, 2016

No description provided.

@honest-hrundel
Copy link

🍅 Не пройден линтинг или базовые тесты

@honest-hrundel
Copy link

🍅 Не пройден линтинг или базовые тесты

@honest-hrundel
Copy link

🍅 Пройдено тестов 14 из 16

@honest-hrundel
Copy link

🍅 Не пройден линтинг или базовые тесты

@honest-hrundel
Copy link

🍅 Пройдено тестов 14 из 16

@honest-hrundel
Copy link

🍅 Не пройден линтинг или базовые тесты

@honest-hrundel
Copy link

🍅 Пройдено тестов 14 из 16

@honest-hrundel
Copy link

🍅 Не пройден линтинг или базовые тесты

@honest-hrundel
Copy link

🍅 Не пройден линтинг или базовые тесты

@honest-hrundel
Copy link

🍅 Пройдено тестов 14 из 16

@honest-hrundel
Copy link

🍅 Пройдено тестов 14 из 16

@honest-hrundel
Copy link

🍅 Не пройден линтинг или базовые тесты

@honest-hrundel
Copy link

🍏 Пройдено тестов 16 из 16


var BANK_TIMEZONE = 0; // временная зона банка
var MINUTES_IN_DAY = 24 * 60;
var DATE_PATTERN = /(([БВНПРСТЧ]+) )?(\d+):(\d+)([+-]\d+)/i;
Copy link

Choose a reason for hiding this comment

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

сам придумал?

Danny: [],
Rusty: [],
Linus: [],
Gang: []
Copy link

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) {
Copy link

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);
Copy link

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]);
Copy link

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);
Copy link

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;
Copy link

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) {
Copy link

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) {
Copy link

Choose a reason for hiding this comment

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

тоже самое с "волшебными" 3 и 1

@Lakate
Copy link

Lakate commented Oct 30, 2016

привет!
посмотрела решение. За комментарии просто 🌟. Но есть замечания, которые надо поправить.
Как поправишь, пиши в слак - @Lakate. Будем дальше смотреть)

@Lakate
Copy link

Lakate commented Oct 30, 2016

🍅

@honest-hrundel
Copy link

🍏 Пройдено тестов 16 из 16

@honest-hrundel
Copy link

🍏 Пройдено тестов 16 из 16

@honest-hrundel
Copy link

🍅 Не пройден линтинг или базовые тесты

@honest-hrundel
Copy link

🍅 Пройдено тестов 10 из 16

@honest-hrundel
Copy link

🍅 Не пройден линтинг или базовые тесты

@honest-hrundel
Copy link

🍅 Пройдено тестов 10 из 16

@honest-hrundel
Copy link

🍅 Пройдено тестов 10 из 16

@honest-hrundel
Copy link

🍅 Не пройден линтинг или базовые тесты

@honest-hrundel
Copy link

🍅 Не пройден линтинг или базовые тесты

@honest-hrundel
Copy link

🍅 Пройдено тестов 10 из 16

@honest-hrundel
Copy link

🍅 Не пройден линтинг или базовые тесты

@honest-hrundel
Copy link

🍅 Пройдено тестов 10 из 16

@honest-hrundel
Copy link

🍅 Не пройден линтинг или базовые тесты

@honest-hrundel
Copy link

🍏 Пройдено тестов 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;
Copy link

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; // временная зона банка
Copy link

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);
// Из готовых диапазонов получаем время, когда можно ограбить банк
// с учётом продолжительности ограбления (если вообще можно)
Copy link

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++) {
Copy link

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];
Copy link

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 нет
Copy link

Choose a reason for hiding this comment

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

вообще непонятный комментарий. Он совсем запутывает. И на самом деле вот это сомнительное место
getFreeIntervals и getBusyIntervals. Подумай как оптимизировать это?

@Lakate
Copy link

Lakate commented Nov 23, 2016

🍅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants