Skip to content

Conversation

@MKruchok
Copy link
Owner

No description provided.

def downwards(self, index):
next_index = index + 1
if next_index is len(self.heap):
def up(self, index, elem):

Choose a reason for hiding this comment

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

Try to name your functions with a verb (because the function does some action). In this case, you could call your up function as swim, bubble_up, etc. Avoid such namings as up / upwards etc.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Fixed.

if next_index is len(self.heap):
def up(self, index, elem):
while index > 0 and elem.priority > self.heap[int((index - 1) / 2)].priority:
self.heap[int(index)] = self.heap[int((index - 1) / 2)]

Choose a reason for hiding this comment

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

What is int((index - 1) / 2 in your function? You've already used it twice here, so consider saving this expression to a variable with readable naming that explains the purpose of this expression.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Better implemented now.

if next_index is len(self.heap):
def up(self, index, elem):
while index > 0 and elem.priority > self.heap[int((index - 1) / 2)].priority:
self.heap[int(index)] = self.heap[int((index - 1) / 2)]

Choose a reason for hiding this comment

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

Why do you wrap index with int? It's already int.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Sometimes indexes is float. For example: when index is 2, 2 - 1 is 1, 1 / 2 is 0,5. Indexes must be int, not float. int() function returns a number rounded + converts it to int.

Copy link
Owner Author

@MKruchok MKruchok Sep 23, 2021

Choose a reason for hiding this comment

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

Better implemented now.

while index > 0 and elem.priority > self.heap[int((index - 1) / 2)].priority:
self.heap[int(index)] = self.heap[int((index - 1) / 2)]
index -= 1
index /= 2

Choose a reason for hiding this comment

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

Why did you write these operations separately? It's actually index = (index - 1) / 2 (you have used it already). So what this expression means?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Fixed

else:
self.heap[int(index)] = elem

def down(self, index):

Choose a reason for hiding this comment

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

Again. Name your functions with a verb. In this case, you could name your down function as sink, bubble_down, etc.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Fixed

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.

3 participants