Skip to content

Melanin Nikita #28

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

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from
Open

Conversation

NikitaMelanin
Copy link

No description provided.

@sadyan42 sadyan42 assigned loozmax and unassigned sadyan42 Oct 5, 2021

}
};
Copy link

Choose a reason for hiding this comment

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

2 балла

@@ -9,6 +9,8 @@
*/

function Organaizer(meetings = [], vacations = []) {
this.meetings = meetings;
Copy link

Choose a reason for hiding this comment

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

Проверка на входные данные?

Copy link
Author

Choose a reason for hiding this comment

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

У нас же указано, что поступает: "массив данных", зачем делать проверку?

Copy link

Choose a reason for hiding this comment

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

Затем, что мне ничего не запрещает прокинуть туда что-то иное.

@@ -17,6 +17,17 @@
@param {number} minutes - Минуты
*/
function Time(hours, minutes) {
}
if (hours < 0 || hours > 24 || minutes < 0 || minutes > 60) {
Copy link

Choose a reason for hiding this comment

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

А если строка

Copy link
Author

Choose a reason for hiding this comment

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

А так разве изначально не понятно, что будет введено число?

@@ -17,6 +17,15 @@
@param {Time} endTime - Время конца встречи
*/
function Meeting(meetingDate, startTime, endTime) {
}
if (startTime.hours < 8 || endTime.hours > 19 || startTime > endTime){
Copy link

Choose a reason for hiding this comment

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

А если строка

this.endTime = endTime;
};
Meeting.prototype.isMeetingInTimeRange = function(start, end) {
return start.isEarlier(this.endTime) && end.isLater(this.startTime);
Copy link

Choose a reason for hiding this comment

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

Не все возможные случаи

@loozmax
Copy link

loozmax commented Oct 5, 2021

В принципе ок, поправь замечания

@@ -17,6 +17,17 @@
@param {Time} endTime - Время конца встречи
*/
function Meeting(meetingDate, startTime, endTime) {
}
const {Time} = require("../task-1");
Copy link

Choose a reason for hiding this comment

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

все импорты в начале файла

@@ -9,6 +9,11 @@
*/

function Organaizer(meetings = [], vacations = []) {
if(!Array.isArray(meetings) && !Array.isArray(vacations)) {
Copy link

Choose a reason for hiding this comment

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

Необходимо сделать проверку, что массив meetings принадлежит инстансу класса Meeting, а vacations инстанс Vacation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants