Трофимов Павел by tgkd · Pull Request #43 · urfu-2016/javascript-task-3 · GitHub

No description provided.

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

@bazhenova обрати внимание решено доп. задание

Зачем ты удалил все JsDoc? Давай их вернем обратно. Хорошей практикой считается документировать публичные методы, так как по параметрам сложно догадаться, что они содержат

Давай назовем как-нибудь по-другому. Задумайся, какие это дни?

у parseInt лучше указывать вторым аргументом основание системы счисления, чтобы избежать ошибок

Непонятно, что за интервал ты хочешь получить

давай разобьем эту строчку: сначала ты парсишь - это одна переменная, а потом на ней вызывай метод sort - тебе не придется создавать новую переменную, так как данный метод изменяет исходный массив

Надо переименовать, неочевидно название. Первый интервал чего?

Полное расписание? Подумай, как по-другому это можно назвать

Здесь и в строчке выше надо переименовать переменные, они должны отражать смысл

Надо переименовать, непонятно, что за сравнение интервалов. По переменной должно быть понятно, что в ней лежит

Укажи второй аргумент, как я писала выше

Может быть не getMomentForAttack, а getMomentsForRobbery? :) + не забывай, что ты возвращаешь массив, поэтому в названии присутствует множественное число

Вынеси 60 * 1000 в константу

Непонятно, что за интервал

может тогда уж freeTimes, раз это массив

Ого, какое-то 'обнуление' переменной. Можешь пояснить, что ты имел в виду?

На вход приходит численное значение? Может тогда лучше написать так:

return (time < 10 ? '0' : '') + time;

60 * 1000 -> константа

Тут же массив? Значит, robberyTimes

Когда идет вызов нескольких функций подряд, то лучше каждую функцию начинать с новой строки. Перенеси этот replace на новую строчку. Код станет и красивее и читабельнее)

30 -> константа

почему именно больше 1, почему не больше 0?

🍅

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

@bazhenova обрати внимание решено доп. задание

В названии не отражено, на что именно ты проверяешь данный интервал. Раз ты возвращаешь булевское значение, то может быть лучше назвать как-нибудь isIntersected или isIncluded или что-то в этом роде (посмотри, что больше подходит)

Если я правильно поняла, то compareTimes - это isRobberyTime

Лучше не сокращать слова в названиях: timesOfRobbery либо robberyTimes

И еще немного комментариев)

🍅

Ты будешь доделывать задачу?

@bazhenova, да, в процессе. Не знаю как правильно переименовать переменные.

@tgkd Скажи, с чем проблемы, лучше напиши в slack, помогу

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

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

@bazhenova обрати внимание решено доп. задание

еще бы вот тут переименовать, ты же выбираешь максимум из дат, как я поняла; то есть по сути позднее из двух времен. И раз ты возвращаешь дату, то можно назвать getLaterTime. В аргументы передавай firstTime и secondTime

Ну или getLaterDate

Еще пару переименований сделай, и я отдам тебя ментору. Вроде что-то хочется поправить, но не могу понять, что именно. Посмотрим, что скажет ментор

🍅

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

@bazhenova обрати внимание решено доп. задание

🚀

Ты определяешь часовой пояс банка, а потом везде его прокидываешь в качестве аргумента. Проще сохранить в переменную

Тут еще пока не отсортировано. Отсортированным становится только на следующем шаге

ROBBERY_DAYS.indexOf(parsedTime[1]) + 1
В условии сказано, что проверять данные не надо, но допустим в расписании появится ЧТ. ЧТ, как день, является валидным значением, но твой код вернет 0(воскресенье), вместо 4(четверга).

Получается, что шаг в лево и твое решение перестанет работать. Надо переписать на что-то более надежное

Почему тут for, а везде выше forEach?

'ПН 00:00+' - надо унести в константы.

А в каком случае может отработать это условие? Ведь если у интервала начало и конец совпадают, то это уже не интервал, а момент времени.

🍅

Тут все интервалы сравниваются с другими интервалами, но это не очень хорошо с точки зрения производительности. Зачем сравнивать время работы банка в понедельник с свободным временем грабителей во вторник или среду. Попробуй как-нибудь оптимизировать проверку по дням

🍅

Последняя активность 7-го ноября

init

53099ef

honest-hrundel assigned bazhenova Oct 26, 2016

bazhenova reviewed Oct 27, 2016

View reviewed changes

изменения по комментариям

7b242b0

bazhenova reviewed Oct 29, 2016

View reviewed changes

изменены названия переменных

81d99a7

lint

df9c5d0

bazhenova reviewed Nov 2, 2016

View reviewed changes

lint

e5b889a

honest-hrundel assigned meded90 and unassigned bazhenova Nov 2, 2016

gogoleff unassigned meded90 Nov 2, 2016

honest-hrundel assigned ulitcos Nov 2, 2016

ulitcos added the extra label Nov 5, 2016

ulitcos reviewed Nov 5, 2016

View reviewed changes

ulitcos assigned mokhov Dec 12, 2016

ulitcos removed their assignment Dec 12, 2016

-               * @param {String} workingHours.to – Время закрытия, например, "18:00+5"
-               * @returns {Object}
-               */
+              var days = ['ПН', 'ВТ', 'СР'];

+                  var timeRegExp = /([А-Я]{2}) (\d{2}):(\d{2})\+(\d+)/g;
+                  var parsedTime = timeRegExp.exec(date);
+                  var day = days.indexOf(parsedTime[1]) + 1;
+                  var hour = parsedTime[2] - (parseInt(parsedTime[4]) - bankTimezone);

+              function isCrossed(bankInterval, freeInterval) {
+                  var checkFrom = checkInterval(bankInterval.from, freeInterval);
+                  var checkTo = checkInterval(bankInterval.to, freeInterval);
+                  var compareIntervals = freeInterval.from > bankInterval.from &&

+                  var bankTimezone = getBankTimezone(workingHours.from);
+                  var freeTimeIntervals = getSotredIntervals(schedule, bankTimezone);
+                  var bankSchedule = getBankSchedule(workingHours, bankTimezone);
+                  var robberyTime = getMomentForAttack(freeTimeIntervals, bankSchedule, duration);

-                       */
                       tryLater: function () {
+                          if (this.exists()) {
+                              var laterTime = createLaterTime(robberyTime[0].from, 30);



		function getSotredIntervals(schedule, bankTimezone) {
		var sortedSchedule = parseSchedule(schedule, bankTimezone).sort(function (a, b) {

+              function joinSchedule(schedule) {
+                  var fullSchedule = [];
+                  var firstInterval = schedule[0];

+              function getBankTimezone(time) {
+                  var bankTimezone = /\+(\d+)/.exec(time)[1];
+                  return parseInt(bankTimezone);

+                                  to: new Date(Math.min(bankInterval.to, freeInterval.to))
+                              };
+                              var laterTime = new Date(crossInterval.from.getTime() + (duration * 60 * 1000));

+              function getFreeSchedule(busyTime, bankTimezone) {
+                  var freeTime = [];
+                  var interval = {};

+                      }
+                      interval.to = currentInterval.from;
+                      freeTime.push(interval);
+                      interval = {};


		function formatTime(time) {

		return (time.toString().length === 2 ? '' : '0') + time.toString();


		function createLaterTime(date, shift) {

		return new Date(date.getTime() + (shift * 60 * 1000));


		var time = robberyTime[0].from;

		return template.replace('%HH', formatTime(time.getHours()))

+                                  robberyTime[0].from = laterTime;
+                                  return true;
+                              } else if (robberyTime.length > 1) {

		@@ -1,49 +1,198 @@
		'use strict';

		/**

		}


		function getInterval(from, to, timezone) {

+                  return getFreeSchedule(joinedSchedule, bankTimezone);
+              }
+              function checkInterval(time, interval) {

+              function isCrossedIntervals(bankInterval, freeInterval) {
+                  var checkTimeFrom = checkInterval(bankInterval.from, freeInterval);
+                  var checkTimeTo = checkInterval(bankInterval.to, freeInterval);
+                  var compareTimes = freeInterval.from > bankInterval.from &&

+              }
+              function getMomentsForRobbery(freeSchedule, bankSchedule, duration) {
+                  var timesToRob = [];

+                  return interval.from <= time && time <= interval.to;
+              }
+              function createDate(currentTime, firstTime) {



		function getSortedIntervals(schedule, bankTimezone) {
		var sortedSchedule = parseSchedule(schedule, bankTimezone);

+                          busyIntervals.push(totalBusyInterval);
+                          totalBusyInterval = schedule[i];
+                      }
+                  }

+                  var freeIntervals = [];
+                  var currentFreeInterval = {};
+                  currentFreeInterval.from = getNewDate('ПН 00:00+' + bankTimezone, bankTimezone);

+                  busyTime.forEach(function (currentBusyInterval) {
+                      if (currentBusyInterval.from === currentBusyInterval.to) {
+                          return;
+                      }

+                              }
+                          }
+                      });
+                  });

Трофимов Павел by tgkd · Pull Request #43 · urfu-2016/javascript-task-3 · GitHub

Uh oh!

Трофимов Павел - #43

Трофимов Павел#43tgkd wants to merge 5 commits intourfu-2016:masterurfu-2016/javascript-task-3:masterfrom tgkd:masterCopy head branch name to clipboard

Conversation

tgkd commented Oct 26, 2016

Uh oh!

honest-hrundel commented Oct 26, 2016

Uh oh!

honest-hrundel commented Oct 26, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bazhenova commented Oct 27, 2016

Uh oh!

honest-hrundel commented Oct 28, 2016

Uh oh!

honest-hrundel commented Oct 28, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bazhenova commented Oct 29, 2016

Uh oh!

bazhenova commented Oct 29, 2016

Uh oh!

bazhenova commented Nov 2, 2016

Uh oh!

tgkd commented Nov 2, 2016

Uh oh!

bazhenova commented Nov 2, 2016

Uh oh!

honest-hrundel commented Nov 2, 2016

Uh oh!

honest-hrundel commented Nov 2, 2016

Uh oh!

honest-hrundel commented Nov 2, 2016

Трофимов Павел#43
tgkd wants to merge 5 commits into
urfu-2016:masterfrom
tgkd:master