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

Чернышёв Александр #42

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
166 changes: 162 additions & 4 deletions robbery.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,140 @@ exports.isStar = true;
* @returns {Object}
*/
exports.getAppropriateMoment = function (schedule, duration, workingHours) {
console.info(schedule, duration, workingHours);

var minPerDay = 1440;
Copy link

@ninjagrizzly ninjagrizzly Nov 1, 2016

Choose a reason for hiding this comment

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

Это константы, их нужно вынести из функции и именовать согласно принятым стандартам CONST_HUNDRED = 100

var minPerHour = 60;

var answerArray = getAnswerArray();

var answer = answerArray.length === 0 ? -1 : answerArray[0][0];

Choose a reason for hiding this comment

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

Странное название переменной. Если есть ответ зачем что-то еще искать?)


var arrayOfGoodTime = getArrOfGoodTime();

var tryLaterCounter = 0;

function getArrOfGoodTime() {
Copy link

@ninjagrizzly ninjagrizzly Nov 1, 2016

Choose a reason for hiding this comment

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

Хотелось бы увидеть комментарии к каждой функции, что на входе и что она делает со своими входными данными (т.е. этот комментарий относится ко всем функциям, а не конкретно к этой)

if (answer < 0) {
return [];
}
var resultArray = [];
var arr = answerArray;

Choose a reason for hiding this comment

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

Зачем answerArray пихать в arr

var ans = answer;
for (var i = 0; i < arr.length; i ++) {
ans = arr[i][0];
while (ans + duration <= arr[i][1]) {
resultArray.push(ans);
ans += 30;
}
}
resultArray.splice(0, 1);

return resultArray;
}

function getBusyIntervals() {

var arrIntervals = [];

for (var friend in schedule) {

Choose a reason for hiding this comment

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

Используй Object.keys(schedule) там встроенный hasOwnProperty

if ({}.hasOwnProperty.call(schedule, friend)) {
arrIntervals = arrIntervals.concat(getArrayOfBusyInterval(schedule[friend]));
}
}

var bankStartWork = getTimestamp(workingHours.from);
var bankFinishWork = getTimestamp(workingHours.to);
arrIntervals.push([0, bankStartWork], [bankFinishWork, bankStartWork + minPerDay],

Choose a reason for hiding this comment

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

А если банк начинает работу с 0 часов? тогда в busyIntervals появится [0, 0]. В этом случае программа отработает правильно?

[bankFinishWork + minPerDay, bankStartWork + minPerDay * 2],
[bankFinishWork + minPerDay * 2, minPerDay * 3 - 1]);

return arrIntervals;
}

function getAnswerArray() {
var busyIntervals = getBusyIntervals();

var sortAndMergeArray = mergeRange(busyIntervals);

Choose a reason for hiding this comment

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

просто mergedArray


var start = sortAndMergeArray[0][1];
var finish = 0;
var resArray = [];
for (var i = 1; i < sortAndMergeArray.length; i++) {
finish = sortAndMergeArray[i][0];
if (finish - start >= duration) {
resArray.push([start, finish]);
}
start = sortAndMergeArray[i][1];
}

return resArray;
}


function getArrayOfBusyInterval(scheduleArray) {
var timeArray = [];
for (var i = 0; i < scheduleArray.length; i++) {
timeArray.push(
[getTimestamp(scheduleArray[i].from),
getTimestamp(scheduleArray[i].to)
]);
}

return timeArray;
}

function sortArray(array) {
return array.sort(function compareArrays(a, b) {

Choose a reason for hiding this comment

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

Необязаттельно давать этой функции имя compareArrays, эту функцию ты по имени не вызываешь, она вполне может быть анонимной

return a[0] - b[0];
});
}

function mergeRange(array) {
var sortedArray = sortArray(array);
var resultArray = [];
var prevStart = sortedArray[0][0];
var prevFinish = sortedArray[0][1];
for (var i = 1; i < sortedArray.length; i++) {
if (sortedArray[i][0] <= prevFinish) {
prevFinish = Math.max(sortedArray[i][1], prevFinish);
} else {
resultArray.push([prevStart, prevFinish]);
prevStart = sortedArray[i][0];
prevFinish = sortedArray[i][1];
}
}
resultArray.push([prevStart, prevFinish]);

return resultArray;

}

function getUTC(stringTime) {
return Number(stringTime.substring(stringTime.length - 1));

Choose a reason for hiding this comment

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

А если будет +10 ? тогда вернет 0, неверно

}

function getTimestamp(stringTime) {
var mainUTC = getUTC(workingHours.from);
if (stringTime.length === 7) {

Choose a reason for hiding this comment

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

Проверка длины строки не лучшая идея, легко описать случаи, когда программа поведет себя неправильно

Copy link

@ninjagrizzly ninjagrizzly Nov 1, 2016

Choose a reason for hiding this comment

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

Длина строки времени работы банка может быть равна 8, в случае если сдвиг больше +9
Для вычленения минут/часов/дней я бы использовал regexp

Choose a reason for hiding this comment

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

Кстати я обращал внимание на это еще в первом review

var h = Number(stringTime.substring(0, 2));
var m = Number(stringTime.substring(3, 5));

return m + minPerHour * h;
}
var day = 0;

Choose a reason for hiding this comment

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

var DAYS = ['ПН', 'ВТ', 'СР', 'ЧТ', 'ПТ', 'CБ', 'ВС'];
var day = DAYS[stringTime.substring(0, 2)];

Итого 2 строки вместо 6

Choose a reason for hiding this comment

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

И это тоже я уже писал

if (stringTime.substring(0, 2) === 'ВТ') {
day = minPerDay;
} else if (stringTime.substring(0, 2) === 'СР') {

Choose a reason for hiding this comment

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

Для вычленения частей даты проще написать одно регулярное выражение, чем каждый раз искать substring.
Попробуй реализовать через него

day = minPerDay * 2;
}
var UTC = Number(stringTime.substring(stringTime.length - 1));
var hours = Number(stringTime.substring(3, 5));
var minuts = Number(stringTime.substring(6, 8));

return minuts + minPerHour * hours + minPerHour * (mainUTC - UTC) + day;

}


return {

Expand All @@ -24,7 +157,7 @@ exports.getAppropriateMoment = function (schedule, duration, workingHours) {
* @returns {Boolean}
*/
exists: function () {
return false;
return answer >= 0;
},

/**
Expand All @@ -35,7 +168,26 @@ exports.getAppropriateMoment = function (schedule, duration, workingHours) {
* @returns {String}
*/
format: function (template) {
return template;
if (!this.exists()) {
return '';
}
var day = Math.floor(answer / minPerDay);
var hours = Math.floor((answer - day * minPerDay) / minPerHour);
var minutes = answer - day * minPerDay - hours * minPerHour;

var days = ['ПН', 'ВТ', 'СР'];

Choose a reason for hiding this comment

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

В константы можно вынести

var dayStr = days[day];

if (minutes < 10) {
minutes = '0' + minutes;
}
if (hours < 10) {
hours = '0' + hours;
}

return template.replace(/%DD/, dayStr)
.replace(/%HH/, hours)
.replace(/%MM/, minutes);
},

/**
Expand All @@ -44,7 +196,13 @@ exports.getAppropriateMoment = function (schedule, duration, workingHours) {
* @returns {Boolean}
*/
tryLater: function () {
return false;
if (!this.exists() || arrayOfGoodTime.length === tryLaterCounter) {
return false;
}
answer = arrayOfGoodTime[tryLaterCounter];
tryLaterCounter++;

return true;
}
};
};