Skip to main content
Code Review

Return to Answer

added 66 characters in body; added 2 characters in body
Source Link
Graipher
  • 41.6k
  • 7
  • 70
  • 134

Your method isEmpty is not needed at all. First, none of your code uses it. Second, if you were to keep it, you should turn it's logic around and rename it to __bool__, this way, if heap automatically works and is True if the heap is not empty. But third, if no __bool__ method exists, Python already calls len(obj) != 0 to determine an object's truthiness, so it is already supplied by defining __len__.

As a second point, you should not mix naming conventions. You currently use both lower_case and camelCase for youyour​ method names. PEP8, Python's official style-guide, recommends using lower_case.

You don't need the line-continuation in here, the line is already shorter than 80 characters:

 def _swap(self, i, j):
 self._data[i], self._data[j] = self._data[j], self._data[i]

Some docstrings describing what your methods do and what they expect as inputs/what they return, would also be really good.

I would rename your method del_min to pop_min, because it returns the removed element and not just deletes it.

Your method isEmpty is not needed at all. First, none of your code uses it. Second, if you were to keep it, you should rename it to __bool__, this way, if heap automatically works. But third, if no __bool__ method exists, Python already calls len(obj) != 0 to determine an object's truthiness, so it is already supplied by defining __len__.

As a second point, you should not mix naming conventions. You currently use both lower_case and camelCase for you method names. PEP8, Python's official style-guide, recommends using lower_case.

You don't need the line-continuation in here, the line is already shorter than 80 characters:

 def _swap(self, i, j):
 self._data[i], self._data[j] = self._data[j], self._data[i]

Some docstrings describing what your methods do and what they expect as inputs/what they return, would also be really good.

I would rename your method del_min to pop_min, because it returns the removed element and not just deletes it.

Your method isEmpty is not needed at all. First, none of your code uses it. Second, if you were to keep it, you should turn it's logic around and rename it to __bool__, this way, if heap automatically works and is True if the heap is not empty. But third, if no __bool__ method exists, Python already calls len(obj) != 0 to determine an object's truthiness, so it is already supplied by defining __len__.

As a second point, you should not mix naming conventions. You currently use both lower_case and camelCase for your​ method names. PEP8, Python's official style-guide, recommends using lower_case.

You don't need the line-continuation in here, the line is already shorter than 80 characters:

 def _swap(self, i, j):
 self._data[i], self._data[j] = self._data[j], self._data[i]

Some docstrings describing what your methods do and what they expect as inputs/what they return, would also be really good.

I would rename your method del_min to pop_min, because it returns the removed element and not just deletes it.

deleted 9 characters in body
Source Link
Graipher
  • 41.6k
  • 7
  • 70
  • 134

Your method isEmpty is not needed at all. First, none of your code uses it. Second, if you were to keep it, you should rename it to __bool__, this way, if heap automatically works. But third, if no __bool__ method exists, Python already calls len(obj) != 0 to determine an object's truthiness, so it is already supplied by defining __len__.

As a second point, you should not mix naming conventions. You currently use both lower_case and camelCase for you method names. PEP8, Python's official style-guide, recommends using lower_case.

Finally, youYou don't need the line-continuation in here, the line is already shorter than 80 characters:

 def _swap(self, i, j):
 self._data[i], self._data[j] = self._data[j], self._data[i]

Some docstrings describing what your methods do and what they expect as inputs/what they return, would also be really good.

I would rename your method del_min to pop_min, because it returns the removed element and not just deletes it.

Your method isEmpty is not needed at all. First, none of your code uses it. Second, if you were to keep it, you should rename it to __bool__, this way, if heap automatically works. But third, if no __bool__ method exists, Python already calls len(obj) != 0 to determine an object's truthiness, so it is already supplied by defining __len__.

As a second point, you should not mix naming conventions. You currently use both lower_case and camelCase for you method names. PEP8, Python's official style-guide, recommends using lower_case.

Finally, you don't need the line-continuation in here, the line is already shorter than 80 characters:

 def _swap(self, i, j):
 self._data[i], self._data[j] = self._data[j], self._data[i]

Some docstrings describing what your methods do and what they expect as inputs/what they return, would also be really good.

I would rename your method del_min to pop_min, because it returns the removed element and not just deletes it.

Your method isEmpty is not needed at all. First, none of your code uses it. Second, if you were to keep it, you should rename it to __bool__, this way, if heap automatically works. But third, if no __bool__ method exists, Python already calls len(obj) != 0 to determine an object's truthiness, so it is already supplied by defining __len__.

As a second point, you should not mix naming conventions. You currently use both lower_case and camelCase for you method names. PEP8, Python's official style-guide, recommends using lower_case.

You don't need the line-continuation in here, the line is already shorter than 80 characters:

 def _swap(self, i, j):
 self._data[i], self._data[j] = self._data[j], self._data[i]

Some docstrings describing what your methods do and what they expect as inputs/what they return, would also be really good.

I would rename your method del_min to pop_min, because it returns the removed element and not just deletes it.

Source Link
Graipher
  • 41.6k
  • 7
  • 70
  • 134

Your method isEmpty is not needed at all. First, none of your code uses it. Second, if you were to keep it, you should rename it to __bool__, this way, if heap automatically works. But third, if no __bool__ method exists, Python already calls len(obj) != 0 to determine an object's truthiness, so it is already supplied by defining __len__.

As a second point, you should not mix naming conventions. You currently use both lower_case and camelCase for you method names. PEP8, Python's official style-guide, recommends using lower_case.

Finally, you don't need the line-continuation in here, the line is already shorter than 80 characters:

 def _swap(self, i, j):
 self._data[i], self._data[j] = self._data[j], self._data[i]

Some docstrings describing what your methods do and what they expect as inputs/what they return, would also be really good.

I would rename your method del_min to pop_min, because it returns the removed element and not just deletes it.

lang-py

AltStyle によって変換されたページ (->オリジナル) /