Skip to content

Gloria #52

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Gloria #52

wants to merge 3 commits into from

Conversation

CheezItMan
Copy link

Gloria,

You didn't actually make a PR instead you sent in a link to the screen before submitting the PR...

@CheezItMan CheezItMan closed this Dec 11, 2021
@CheezItMan CheezItMan reopened this Dec 11, 2021
Copy link
Author

@CheezItMan CheezItMan left a comment

Choose a reason for hiding this comment

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

Overall nice work, you hit the main learning goals here. Well done.

Comment on lines +13 to +16
self.head = None #keep the head private.Not accessible outside class
self.len = 0
self.tail = None
self.max = 0
Copy link
Author

Choose a reason for hiding this comment

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

You're not actually using the tail etc

Comment on lines 24 to 26
# Time Complexity: ?
# Space Complexity: ?
def get_first(self):
Copy link
Author

Choose a reason for hiding this comment

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

👍 Time/space complexity?

Comment on lines 35 to 37
# Time Complexity: ?
# Space Complexity: ?
def add_first(self, value):
Copy link
Author

Choose a reason for hiding this comment

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

👍 Time/space complexity?

# next_node = self.head
self.head = Node(value, next_node = self.head)
self.len += 1
self.update_max(value)
Copy link
Author

Choose a reason for hiding this comment

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

Nice little helper

Comment on lines 46 to 48
# Time Complexity: ?
# Space Complexity: ?
def search(self, value):
Copy link
Author

Choose a reason for hiding this comment

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

👍 Time/space complexity?

Comment on lines +79 to +80

if counter == index:
Copy link
Author

Choose a reason for hiding this comment

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

Suggested change
if counter == index:
if counter == index:

Comment on lines 88 to 90
# Time Complexity: ?
# Space Complexity: ?
def get_last(self):
Copy link
Author

Choose a reason for hiding this comment

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

👍 Time/space complexity?

Comment on lines +103 to +107
if self.len == 0:
self.tail = Node(value, next_node = None)
self.head = self.tail
self.update_max(value)

Copy link
Author

Choose a reason for hiding this comment

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

You only really use tail here, but you would potentially need to update it in add_first and remove_first etc.

@@ -89,7 +163,24 @@ def visit(self):
# Time Complexity: ?
# Space Complexity: ?
def reverse(self):
Copy link
Author

Choose a reason for hiding this comment

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

Just noting this is empty.

Comment on lines 128 to 130
# Time Complexity: ?
# Space Complexity: ?
def delete(self, value):
Copy link
Author

Choose a reason for hiding this comment

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

👍 Time/space complexity?

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