-
Notifications
You must be signed in to change notification settings - Fork 5
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
Rock - Aeron Roemer #1
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work Aeron, you really got into things quickly and you hit the main learning goals here. I made some suggestions and take a look at my feedback and let me know what questions you have.
if (this.front == 0 && (this.rear == this.store.size - 1)){ | ||
return false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This if
condition will prevent the rear from wrapping around the buffer, which is the behavior you want.
I think it should instead be:
if (this.front == 0 && (this.rear == this.store.size - 1)){ | |
return false | |
if ((this.rear +1) % this.store.size == this.front){ | |
return false |
this.rear = (this.rear + 1) % this.size | ||
this.store[this.rear] = element | ||
} | ||
|
||
} | ||
|
||
dequeue() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
} | ||
|
||
front() { | ||
throw new Error("This method has not been implemented!"); | ||
frontItem() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
} | ||
|
||
size() { | ||
throw new Error("This method has not been implemented!"); | ||
sizeOfQ() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 , although since this size
method is a member of the Queue
instance, the "Q" is kind of assumed.
} | ||
|
||
pop() { | ||
throw new Error("This method has not been implemented!"); | ||
if (!this.store.head) return null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could also use the linked list empty method.
throw new Error("This method has not been implemented!"); | ||
if (!this.store.head) return true; | ||
|
||
if (this.store.head.next) return false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could give a false empty when the Stack has 1 element. You can just return false if the prior if statement doesn't exit.
if (this.store.head.next) return false; | |
return false; |
I forked before the changes & used my own linked list from the previous assignment.