-
Notifications
You must be signed in to change notification settings - Fork 12
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
data structures completed #4
base: master
Are you sure you want to change the base?
Conversation
node.next = Node(value, node.next) | ||
self._length+=1 | ||
return | ||
error_message = f'Value {value} is not in the list' |
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.
Did you mean successor
is not in the list?
|
||
def insert(self, value, successor = None): | ||
if(successor and self._root.value != successor): | ||
for node in self: |
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.
Refer to Common Data Structure Operations
table here: https://www.bigocheatsheet.com/
It is common that insert
operation works for constant time. Your solution is O(n).
Please, reconsider implementation and improve working time of insert
method.
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.
Sorry, but no. Insertion in single linked list is O(1) if we insert into the end of the list. But here the condition was to implement insertion before the successor, so we actually have two operations: searching for successor (which is O(n)) and then inserting (which is — after we have found a successor — is actually O(1)).
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.
Dear @eoneoff , according to the table I mentioned earlier, insertion in singly-linked list is O(1) for both average
and worst
cases. The problem here is that successor
has the same data type as value
, but it might be of type Node
. Anyway it is up to you whether improve the working time or not, there is no specific requirement to fit O(1)
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.
Sorry, but that makes on sense. To get the exact Note type object before which we want to insert a new Node with a value from the list, we still need to find it in the list (access complexity O(n)). So if we pass a Node as a successor we still first need to perform a search for this node with complexity O(n). And we do not have any other access to a random node in the list except by iterating the list one-by-one starting from the root node — than's the nature of the linked list.
Is is possible, of course, to separate these two operations and move a search to individual method, so the method for insertion would look like O(1). But the search method, which we will have to run before every insertion before successor will be still O(n), so general performance will be still O(n) too.
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 are right, but
- insert and access are two different operations, and you can split your code into two operations
- having successor of type Node, you already have all the data to perform insert operation, no need to perform a search first
No description provided.