Repository navigation
Conversation
This comment has been minimized.
This comment has been minimized.
pathywang
added
the
Needs Review
cjyuan
left a comment
cjyuan
left a comment
There was a problem hiding this comment.
a bit lost with assignment then with AL help,manage to understand and complete task
That's not a typical Changelist content and it does not convey what was changed in this PR. Could you rephrase it?
| class Node: | ||
| def __init__(self, value): | ||
| self.value = value | ||
| self.next = None | ||
| self.previous = None |
There was a problem hiding this comment.
May I suggest exploring the use of __slots__ to reduce memory usage?
There was a problem hiding this comment.
Thanks for the suggestion! I've added slots to the Node class to reduce per-instance memory overhead.
| if self.head == self.tail: | ||
| self.head = None | ||
| self.tail = None | ||
| else: | ||
| self.tail = node.previous | ||
| self.tail.next = None |
There was a problem hiding this comment.
Could consider delegating the node removing task to remove() -- less code to maintain.
There was a problem hiding this comment.
Thanks, yes I've updated pop_tail() to delegate node removal to remove(), avoiding duplicated logic.
| def remove(self, node): | ||
| if node == self.head and node == self.tail: | ||
| self.head = None | ||
| self.tail = None | ||
|
|
||
| elif node == self.head: | ||
| self.head = node.next | ||
| self.head.previous = None | ||
|
|
||
| elif node == self.tail: | ||
| self.tail = node.previous | ||
| self.tail.next = None | ||
|
|
||
| else: | ||
| node.previous.next = node.next | ||
| node.next.previous = node.previous No newline at end of file |
There was a problem hiding this comment.
Suggestion: Look up why, in a doubly linked List implementation, assigning .next and .previous of the removed node to None a good practice?
There was a problem hiding this comment.
Thanks for the suggestion! I've updated remove() to set the removed node's next and previous references to None, ensuring it is fully detached from the list. I think the reason is that setting both pointers to None makes the removal explicit and prevents the removed node from retaining unnecessary references to neighboring nodes.
cjyuan
added
Reviewed
pathywang
added
Needs Review
|
Changes look good. Well done. |
cjyuan
added
Complete
|
Many thanks, cjyuan. |
2 participants
Self checklist
Task code
CYF-1180
Changelist
Implemented a doubly linked list in Python with O(1) operations for adding elements at the head, removing elements from the tail, and removing specific elements using node handles.