Added meeting scheduler low level design #4
Conversation
|
@thesaltree Design is ready for Merge. |
|
Are we merging this? |
|
@codeimmortal thanks for the PR. Some critical functionalities are missing in the implementation, like:
Can you please add these? |
|
Added mutex , cancelRoom , GetAllFree room functionality. |
| if meetingId > len(ms.meeting) { | ||
| return errors.New("There is no such meeting") | ||
| } |
There was a problem hiding this comment.
@codeimmortal valid index range is [0, len(ms.meeting)-1]. This should be fixed to:
if meetingId >= len(ms.meeting) || meetingId < 0
| users := []*User{ | ||
| &User{id: 1, name: "sam", email: "sam@email.com"}, | ||
| &User{id: 2, name: "ron", email: "ron@email.com"}, | ||
| &User{id: 2, name: "don", email: "don@email.com"}, |
There was a problem hiding this comment.
@codeimmortal multiple users have the same id=2, which would cause issues in participant management.
| name string | ||
| status BookingStatus | ||
| location location | ||
| calender *Calendar |
There was a problem hiding this comment.
@codeimmortal there is inconsistency in naming --> calendar vs calender
|
|
||
| // cancel meeting | ||
| ms.meeting[meetingId].CancelMeeting() | ||
|
|
There was a problem hiding this comment.
After canceling a meeting, it remains in ms.meeting, which could cause unnecessary memory usage. We can set ms.meeting[meetingId] = nil
| func (m *MeetingRoom) isFree(dur *interval) bool { | ||
| if m.calender.isFree(dur) { | ||
| return true | ||
| } | ||
|
|
||
| return false | ||
| } | ||
|
|
||
| // IsFree is use to explose outside | ||
| func (m *MeetingRoom) IsFree(dur *interval) bool { | ||
| m.mu.Lock() | ||
| defer m.mu.Unlock() | ||
|
|
||
| return m.isFree(dur) | ||
| } |
There was a problem hiding this comment.
@codeimmortal this can be refactored to:
func (m *MeetingRoom) IsFree(dur *interval) bool {
m.mu.Lock()
defer m.mu.Unlock()
return m.calender.isFree(dur)
}
|
@codeimmortal thanks for addressing the feedback. Can you also add a section for the readme file? |
What this design contains: