Skip to content

Nicole Washington:Maple#77

Open
N-Washington wants to merge 1 commit intoAda-C16:masterfrom
N-Washington:master
Open

Nicole Washington:Maple#77
N-Washington wants to merge 1 commit intoAda-C16:masterfrom
N-Washington:master

Conversation

@N-Washington
Copy link

Linked List

Copy link

@kyra-patton kyra-patton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Overall good work, Nicole, however there are some comments you should take a look at. It may have been because of the work you lost and the rush to complete that work again. Look over my comments and feel free to reach out with your questions.

🟡

# Space Complexity: ?
# Time Complexity: constant, no traversing needed
# Space Complexity: constant, no new data structures
def get_first(self):

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

# Space Complexity: ?
# Time Complexity: constant
# Space Complexity: constant
def add_first(self, value):

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

# Space Complexity: ?
# Time Complexity: linear
# Space Complexity: constant
def search(self, value):

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if self.head is None:
return False
current_node = self.head
target_node = Node(value)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can create a new node for the target, but it's not necessary. See my suggestion on line 45 ⬇️


# traverse list til reach target node or end of list
while current_node:
if current_node.value == target_node.value:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

current_node.value is an integer, so instead of creating a new node target_node that has value value, you can just compare the current node's value directly to the passed in value.

Suggested change
if current_node.value == target_node.value:
if current_node.value == value:

def add_last(self, value):
pass
if self.head is None:
self.head = value

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 self.head should be equal to a node object, not an integer like value

Comment on lines +122 to +124
new_tail_node = prev_node
current_node.next = new_tail_node
new_tail_node.next = None

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here, after creating the new_tail_node on line 115, you are overwriting it with prev_node which, once the while loop is finished executing, should be the current tail node.

Once the while loop is finished executing current_node is going to be the node after the the current tail, aka None.

Instead, try pointing the current tail (prev_node) to new_tail_node

while current_node:
if current_node.value > max_node.value:
max_node = current_node
current_node = current_node.next

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔You're creating an infinite loop here since you only reassign current_node to the next node in the list if current_node.value > max_node.value

Comment on lines +106 to +107
# Time Complexity: linear
# Space Complexity: constant

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💫 Correct on the time and space complexity, however see my comments below ⬇️

next_node = current_node.next
target_value = value
if current_node.value == target_value:
current_node = current_node.next

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes current_node equal to what was the second node in the list, but it doesn't redirect self.head to point at the second node in the list

Suggested change
current_node = current_node.next
self.head = current_node.next

# Space Complexity: ?
# Time Complexity: linear
# Space Complexity: linear
def visit(self):

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

# Space Complexity: ?
# Time Complexity: linear
# Space Complexity: constant
def reverse(self):

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants