Skip to content

Conversation

@KatySolo
Copy link

No description provided.

@honest-hrundel honest-hrundel changed the title Солодовникова Екатерина Cолодовникова Екатерина Oct 19, 2017
@honest-hrundel
Copy link

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

Copy link

@mokhov mokhov left a comment

Choose a reason for hiding this comment

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

Код невозможно читать, много волшебства и не читаемых функций. Давай начнём исправления с простого – нужно сделать так, чтобы названия функций вызываемых внутри getAppropriateMoment и findPeriod были такими, чтобы можно было понять что происходит
🍅

@@ -0,0 +1,15 @@
{
// Используйте IntelliSense, чтобы узнать о возможных атрибутах.
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;
const DayShift = {
'ПН': 'ВТ',
Copy link

Choose a reason for hiding this comment

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

очень не понятно почему так :)

robbery.js Outdated
*/
exports.isStar = true;
exports.isStar = false;
const DayShift = {
Copy link

Choose a reason for hiding this comment

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

Обычно с большой буквы именуются классы, а не объекты

robbery.js Outdated
* @param {Number} timezone
* @returns {Object}
*/
function evenSchedule(schedule, timezone) {
Copy link

Choose a reason for hiding this comment

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

Название не соответствует описанию

robbery.js Outdated
* @param {Number} timezone
* @returns {Object}
*/
function changeRecord(date, timezone) {
Copy link

Choose a reason for hiding this comment

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

И тоже название не говорящее

return date;
}
let rawDateFrom = parseDate(date.from);
// ['ПН','12','00','5']
Copy link

Choose a reason for hiding this comment

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

Смотри, вот даже то что ты комментарием пишешь результат операции говорит о том, что этот код спустя неделю будет сложно отлазивать. Как можно было бы назвать функцию parseDate, чтобы не запоминать что там будет?

robbery.js Outdated
*/
function parseDate(date) {
// 'ПН 09:00+3'
let day = date.split(':')[0].split(' ')[0];
Copy link

Choose a reason for hiding this comment

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

date.split(:) можно было бы сохранить в переменную, чтобы не делать каждый раз .split

robbery.js Outdated
let newHour = hours + shift;
if (newHour > 24) {
switch (rawDate[0]) {
case 'ПН':
Copy link

Choose a reason for hiding this comment

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

Кажется для чего-то такого у тебя есть странный объект DayShift выше

robbery.js Outdated
let timezone = getTimeZone(workingHours);
schedule = evenSchedule(schedule, timezone);
schedule = cutSchedule(schedule);
schedule = convertScheduleToMinutes (schedule);
Copy link

Choose a reason for hiding this comment

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

И lint'ер не заргулася?

@honest-hrundel
Copy link

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

@mokhov
Copy link

mokhov commented Nov 6, 2017

Пройденных тестов стало меньше

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants