Conversation
|
🍏 Пройдено тестов 19 из 19 |
|
@YuuKohaku обрати внимание: решено доп. задание |
|
🍏 Пройдено тестов 19 из 19 |
|
@YuuKohaku обрати внимание: решено доп. задание |
YuuKohaku
left a comment
YuuKohaku
left a comment
There was a problem hiding this comment.
Математичный подход к решению, хорошая работа.
Местами методы трудно читаются, часто бросаются в глаза переприсвоения переменных и другие мелочи. Пока что помидор.
🍅
| let daysOfWeek = ['ПН', 'ВТ', 'СР', 'ЧТ', 'ПТ', 'СБ', 'ВС']; | ||
| let day = 0; | ||
| if (containsDay) { | ||
| let dayName = timeString.match(/(..)/)[1]; |
There was a problem hiding this comment.
В данном случае регулярное выражение излишне. Лучше substr.
| exports.isStar = true; | ||
|
|
||
| function parseTime(timeString, containsDay) { | ||
| let daysOfWeek = ['ПН', 'ВТ', 'СР', 'ЧТ', 'ПТ', 'СБ', 'ВС']; |
There was a problem hiding this comment.
Везде, где присвоение происходит один раз, имеет смысл использовать const вместо let.
| } | ||
|
|
||
| function convertRobbersSchedule(schedule) { | ||
| let newSchedule = []; |
There was a problem hiding this comment.
Через reduce получится короче и выразительнее.
|
|
||
| function addInterval(intervals, newInterval) { | ||
| let result = []; | ||
| intervals.forEach(interval => { |
| } | ||
|
|
||
| function removeScheduleIntersections(schedule) { | ||
| let unitedSchedule = []; |
There was a problem hiding this comment.
И тут тоже редьюсер напрашивается. )
| */ | ||
| exports.getAppropriateMoment = function (schedule, duration, workingHours) { | ||
| console.info(schedule, duration, workingHours); | ||
| schedule = convertRobbersSchedule(schedule); |
There was a problem hiding this comment.
Создать отдельную переменную в let для переприсвоений или обойтись без переприсвоений. Присваивать значение аргументам функции не стоит.
| if (this.startTimes.length === 0) { | ||
| return ''; | ||
| } | ||
| let daysOfWeek = ['ПН', 'ВТ', 'СР', 'ЧТ', 'ПТ', 'СБ', 'ВС']; |
There was a problem hiding this comment.
(Опционально)
А вот это можно и в константу наверх вынести.
| let daysOfWeek = ['ПН', 'ВТ', 'СР', 'ЧТ', 'ПТ', 'СБ', 'ВС']; | ||
| let time = this.startTimes[this.currentPos] + this.bankTimeZone * 60; | ||
| let day = daysOfWeek[Math.floor(time / (24 * 60))]; | ||
| time = time % (24 * 60); |
There was a problem hiding this comment.
Давай в отдельную переменную. Трудно следить за ходом мысли, когда содержимое переменной постоянно меняет смысл.
| */ | ||
| tryLater: function () { | ||
| return false; | ||
| if (this.currentPos === this.startTimes.length - 1 || this.startTimes.length === 0) { |
There was a problem hiding this comment.
Если this.startTimes.length вынести в отдельную переменную, условие получится короче, а смысл - понятнее. Кроме того, здесь можно использовать this.exists(), опционально.
| day = daysOfWeek.indexOf(dayName); | ||
| } | ||
| let numbers = timeString.match(/(\d+):(\d+)\+(\d+)$/); | ||
| let hours = Number.parseInt(numbers[1]); |
There was a problem hiding this comment.
Для таких целей хватит просто Number.
3 participants
Загрузил в репозиторий 2016-го сначала, сейчас заметил и решил загрузить сюда