-
Notifications
You must be signed in to change notification settings - Fork 270
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
Fix LinkedList removeAt() #98
Conversation
1.added base case for removeAt(1) 2.switched assigment on line #138 where previous and current where pointing to the same node
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.
Hello @TheKingSombrero, thanks for the PR and it's good to see you took an interest. However, there are a few things that you have to see:
- ESLint rules should not break
- Remove methods should always return a node (and the next should be null)
- Please go through the issue details here: removeAt function in linked-list #83
- The removal method should work as an array
Thanks!
} | ||
|
||
previous.next = address.next; | ||
console.log(address.next) |
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.
No console.log()
in the final push to master
passes eslint and matches and enforced strict equality for some reason it wasn't running in node with it
|
||
if (index >= this.length()) { | ||
if (index === 0) { | ||
let returnNode=this.head; |
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.
why this?
let returnNode=this.head; | ||
this.head=this.head.next; | ||
this.size -= 1; | ||
return returnNode; |
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.
return this.head from here
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.
if (!this.head) {
this.head = node;
this.tail = node;
return node;
}
If you do not make it null
it will contain the full list
this.size -= 1; | ||
return returnNode; | ||
} | ||
if (index >= this.size) { |
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.
in linked list we don't keep size
previous = address; | ||
address = address.next; |
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.
better name
1.added base case for removeAt(1)
2.switched assigment on line #138 where previous and current where pointing to the same node