-
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
Козлова Валерия #57
base: master
Are you sure you want to change the base?
Козлова Валерия #57
Changes from all commits
99678f0
e5d2f85
2b51aa0
3061156
b10d369
af12ee9
79354b5
d4f8f54
fdea8b6
332cf19
146978c
51d9e5e
a753198
22966d1
4ef3f09
a6ffd79
0784f5d
fb0caa2
db69cec
d019062
a53fafd
a4a8844
4faba5e
123f6c2
8d4a634
f06c1c8
18aaff8
42465df
2df9932
1712fe0
9a9bfed
5f83296
5d40043
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,7 +4,174 @@ | |
* Сделано задание на звездочку | ||
* Реализовано оба метода и tryLater | ||
*/ | ||
exports.isStar = true; | ||
exports.isStar = false; | ||
|
||
var WEEK = ['', 'ПН', 'ВТ', 'СР', 'ЧТ', 'ПТ', 'СБ', 'ВС']; | ||
var BEGIN_OF_WEEK = new Date (0, 0, 1, 0, 0); | ||
var END_OF_WEEK = new Date (0, 0, 7, 23, 59); | ||
var msInMinute = 60000; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. константы лучше писать в виде (как у тебя выше): |
||
|
||
function leadToDataType(date, bankTimeZone) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. не понял название - функция перегоняет дату в тайм-зону банку - причем может получаться отрицательное число часов - проверяла, что конструктор дат нормально это обрабатывает? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. я бы назвал |
||
var dayOfWeek = date.slice(0, 2); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. лучше бы все эти даты строковые в объекты перегонять функцией-хелпером, чтобы потом не |
||
var currentDay = WEEK.indexOf(dayOfWeek); | ||
var hours = parseInt(date.slice(3, 5), 10); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. во-во и тут |
||
var timeZone = parseInt(date.slice(8), 10); | ||
hours = hours - timeZone + bankTimeZone; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Это уже не просто There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. можно же так: |
||
var minutes = parseInt(date.slice(6, 8), 10); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. магия |
||
|
||
return new Date (0, 0, currentDay, hours, minutes); | ||
} | ||
|
||
function maxDate() { | ||
var dates = [].slice.call(arguments); | ||
dates.sort(function (date1, date2) { | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Тут не нужна пустая строка There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Почему нельзя воспользоваться функцией There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. у меня не работает Math.max для дат There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. А если один из этих способов? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. О, давай через |
||
return (date2 - date1); | ||
}); | ||
|
||
return dates[0]; | ||
} | ||
|
||
function minDate() { | ||
var dates = [].slice.call(arguments); | ||
dates.sort(function (date1, date2) { | ||
|
||
return (date1 - date2); | ||
}); | ||
|
||
return dates[0]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. и тут давай сделаем |
||
} | ||
|
||
function findFreeGangsterTime(schedule, bankTimeZone) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Функция должна занимать не больше 25 строк There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Кажется, что можно сначала просчитать все There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. И попробуй использовать |
||
var freeSchedule = {}; | ||
var keys = Object.keys(schedule); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. зачем Object.keys(schedule).forEach(/* ... */); |
||
keys.forEach(function (key) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
var length = (schedule[key]).length; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. скобки не нужны |
||
freeSchedule[key] = []; | ||
if (length === 0) { | ||
freeSchedule[key] = [{ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. а чего не |
||
from: BEGIN_OF_WEEK, | ||
to: END_OF_WEEK | ||
}]; | ||
} else { | ||
freeSchedule[key].push({ | ||
from: BEGIN_OF_WEEK, | ||
to: leadToDataType (schedule[key][0].from, bankTimeZone) | ||
}); | ||
for (var i = 1; i < length; i++) { | ||
freeSchedule[key].push({ | ||
from: leadToDataType (schedule[key][i - 1].to, bankTimeZone), | ||
to: leadToDataType (schedule[key][i].from, bankTimeZone) | ||
}); | ||
} | ||
freeSchedule[key].push({ | ||
from: leadToDataType (schedule[key][length - 1].to, bankTimeZone), | ||
to: END_OF_WEEK | ||
}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. все как бы правильно, но очень многословно. Задача верная да, нужно инвертировать интервалы. Как тебе такой алгоритм:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. нигде не наврал? |
||
} | ||
}); | ||
|
||
return freeSchedule; | ||
} | ||
|
||
function getGangFreeTime(schedule) { | ||
var freeTime = []; | ||
schedule.Danny.forEach(function (interval1) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Привязка к именам бандитов. И слишком большая вложенность. Такого быть не должно. Нельзя ли просто по ключам пробежаться? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1 так делать совсем нельзя - а если будет сколько угодно бандитов или имена могу быть другими? |
||
schedule.Rusty.forEach(function (interval2) { | ||
schedule.Linus.forEach(function (interval3) { | ||
var begin = maxDate (interval1.from, interval2.from, interval3.from); | ||
var end = minDate (interval1.to, interval2.to, interval3.to); | ||
if (end > begin) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
freeTime.push({ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ты много где пушишь объект с этими полями. Можно вынести в отдельную функцию |
||
from: begin, | ||
to: end | ||
}); | ||
} | ||
}); | ||
}); | ||
}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. чисто математически алгоритм верный - да, надо получить пересечение "свободных" отрезков всех бандитов |
||
freeTime = formateFreeTime (freeTime); | ||
|
||
return freeTime; | ||
} | ||
|
||
function formateFreeTime(freeTime) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. судя по смыслу, аргумент лучше назвать There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. а почему такое название функции? |
||
var res = []; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Лучше названия не сокращать |
||
freeTime.forEach(function (interval) { | ||
var day1 = interval.from.getDate(); | ||
var day2 = interval.to.getDate(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. хм, возможно, лучше назвать |
||
for (var i = day1; i <= Math.min(day2, 3); i++) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. типа не дальше среды? |
||
res.push({ | ||
from: maxDate (new Date (0, 0, i, 0, 0), interval.from), | ||
to: minDate (new Date (0, 0, i, 23, 59), interval.to) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. а что за даты такие? Чтобы стало понятнее, нужно сделать переменные с говорящими именами There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. мы тут пытаемся разложить interval на отрезки по дням? и в итоге будут теже интервалы разложенные по дням - так? |
||
}); | ||
} | ||
}); | ||
|
||
return res; | ||
} | ||
|
||
function leadDateToObject(date) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Что значит это название?) |
||
return { | ||
hours: parseInt(date.slice(0, 2), 10), | ||
minutes: parseInt(date.slice(3, 5), 10) | ||
}; | ||
} | ||
|
||
function getFreeIntervals(freeTime, workingHours, res) { | ||
var startBankWork = leadDateToObject(workingHours.from); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. хм, хитро и неочевидно - ты используешь функцию, которая нужна для перевода строкового представления даты и сдвига в нужную таймзону - без сдвига в таймзону. Лучше тогда сделать две функции и тут звать только превращение в дату |
||
var endBankWork = leadDateToObject(workingHours.to); | ||
var bank = {}; | ||
freeTime.forEach(function (interval) { | ||
var day = interval.from.getDate(); | ||
bank.start = new Date (0, 0, day, startBankWork.hours, startBankWork.minutes); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ты часто используешь конструкцию: |
||
bank.end = new Date (0, 0, day, endBankWork.hours, endBankWork.minutes); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. а зачем нам объект? можно было бы сделать две переменные - |
||
var begin = maxDate (interval.from, bank.start); | ||
var end = minDate (interval.to, bank.end); | ||
if (end > begin) { | ||
res.push({ | ||
from: begin, | ||
to: end | ||
}); | ||
} | ||
}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. опять по смыслу |
||
|
||
return res; | ||
} | ||
|
||
function getTimeForRobbery(timeToRobbery, msInDuration) { | ||
var res = []; | ||
for (var i = 0; i < timeToRobbery.length; i++) { | ||
if (timeToRobbery[i].to - timeToRobbery[i].from >= msInDuration) { | ||
res.push(timeToRobbery[i]); | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. опять var intervals = timeToRobbery.reduce(function(res, interval) {
if (interval.to - interval.from >= msInDuration) {
res.push(interval);
}
return res;
}, []); и в конце можно написать тернарник: |
||
if (res.length === 0) { | ||
return null; | ||
} | ||
|
||
return res; | ||
} | ||
|
||
function toString(number) { | ||
if (number < 10) { | ||
number = '0' + number; | ||
} | ||
number.toString(); | ||
|
||
return number; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. лучше короче: |
||
} | ||
|
||
function formateDate(date) { | ||
var day = date.getDate(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Не проще ли сделать так: |
||
var hours = date.getHours(); | ||
var minutes = date.getMinutes(); | ||
day = WEEK[day]; | ||
hours = toString (hours); | ||
minutes = toString(minutes); | ||
|
||
return [day, hours, minutes]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Нехорошо писать так: var x = rawData;
x = f(x);
x = g(x);
return [..., x, ...]; Типа перекладывать в одну и туже переменную разное содержимое. Сначала часы у тебя просто число, потом форматированная строка. Тут можно переделать так: var day = date.getDate();
// ...
return [WEEK[day], ...]; |
||
} | ||
|
||
/** | ||
* @param {Object} schedule – Расписание Банды | ||
|
@@ -14,8 +181,14 @@ exports.isStar = true; | |
* @param {String} workingHours.to – Время закрытия, например, "18:00+5" | ||
* @returns {Object} | ||
*/ | ||
|
||
exports.getAppropriateMoment = function (schedule, duration, workingHours) { | ||
console.info(schedule, duration, workingHours); | ||
var gmt = parseInt(workingHours.from.slice(5), 10); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ого, а что такое вернет There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. неочевидно совсем, что он вернет сдвиг - выше я писал, что помог бы хелпер |
||
var msInDuration = msInMinute * duration; | ||
var freeSchedule = findFreeGangsterTime (schedule, gmt); | ||
var freeTime = getGangFreeTime (freeSchedule); | ||
var timeForRobbery = getFreeIntervals (freeTime, workingHours, []); | ||
timeForRobbery = getTimeForRobbery (timeForRobbery, msInDuration); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. уже писал почему это плохой паттерн - переменная одна, а в разное время в ней лежит разное |
||
|
||
return { | ||
|
||
|
@@ -24,7 +197,8 @@ exports.getAppropriateMoment = function (schedule, duration, workingHours) { | |
* @returns {Boolean} | ||
*/ | ||
exists: function () { | ||
return false; | ||
|
||
return (timeForRobbery !== null); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. можно без скобок |
||
}, | ||
|
||
/** | ||
|
@@ -35,16 +209,16 @@ exports.getAppropriateMoment = function (schedule, duration, workingHours) { | |
* @returns {String} | ||
*/ | ||
format: function (template) { | ||
return template; | ||
}, | ||
if (!this.exists()) { | ||
return ''; | ||
} | ||
var format = formateDate(timeForRobbery[0].from); | ||
var newTemplate = template | ||
.replace('%DD', format[0]) | ||
.replace('%HH', format[1]) | ||
.replace('%MM', format[2]); | ||
|
||
/** | ||
* Попробовать найти часы для ограбления позже [*] | ||
* @star | ||
* @returns {Boolean} | ||
*/ | ||
tryLater: function () { | ||
return false; | ||
return newTemplate; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. переменная |
||
} | ||
}; | ||
}; |
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.
Ты уверена, что без этих волшебных дат тут не обойтись?