Skip to content

/ Python

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

Remove unnecessary condition in collision_resolution method #2131

Open
wants to merge 1 commit into
base: master
from
Open

Remove unnecessary condition in collision_resolution method #2131

wants to merge 1 commit into from

Conversation

@marcoscannabrava
Copy link
Contributor

marcoscannabrava commented Jun 17, 2020

Describe your change:

Removed unnecessary condition in looping hash inside collision_resolution method.

  • Add an algorithm?
  • Fix a bug or typo in an existing algorithm?
  • Documentation change?

Checklist:

  • I have read CONTRIBUTING.md.
  • This pull request is all my own work -- I have not plagiarized.
  • I know that pull requests will not be merged if they fail the automated tests.
  • This PR only changes one algorithm file. To ease review, please open separate PRs for separate algorithms.
  • All new Python files are placed inside an existing directory.
  • All filenames are in all lowercase characters with no spaces or dashes.
  • All functions and variable names follow Python naming conventions.
  • All function parameters and return values are annotated with Python type hints.
  • All functions have doctests that pass the automated testing.
  • All new algorithms have a URL in its comments that points to Wikipedia or other similar explanation.
  • If this pull request resolves one or more open issues then the commit message contains Fixes: #{$ISSUE_NO}.
@marcoscannabrava
Remove unnecessary condition in collision_resolution method
Loading status checks…
964879f
@mateuszz0000
mateuszz0000 reviewed Jun 23, 2020
View changes
data_structures/hashing/hash_table.py
@@ -47,7 +47,7 @@ def _set_value(self, key, data):
def _collision_resolution(self, key, data=None):
new_key = self.hash_function(key + 1)

while self.values[new_key] is not None and self.values[new_key] != key:
while self.values[new_key] is not None:

This comment has been minimized.

Sign in to view
Copy link
@mateuszz0000

mateuszz0000 Jun 23, 2020

Member

Why this condition is unnecessary?

This comment has been minimized.

Sign in to view
Copy link
@marcoscannabrava

marcoscannabrava Jun 23, 2020

Author Contributor

I might be wrong so I'd appreciate if you could explain it to me or point me to any link.

  • How can a value be equal to a key? If it was, would it be a problem?

This comment has been minimized.

Sign in to view
Copy link
@cclauss

cclauss Jun 23, 2020

Member

d = {0: 0, 1: 1, 2: 2} # is a dict where all keys equal their values.

This comment has been minimized.

Sign in to view
Copy link
@marcoscannabrava

marcoscannabrava Jun 23, 2020

Author Contributor

In this example d = {0: 0, 1: 1, 2: 2} if I want to insert a new value and the hashing function somehow returns key=2.

  • Wouldn't we want to go into the loop to produce a new key so it doesn't collide with the key-value pair 2-2?
  • It wouldn't anyway because "is not None" is sufficient condition, right?
@itsvinayak itsvinayak requested review from cclauss and mateuszz0000 Jun 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cclauss cclauss

@mateuszz0000 mateuszz0000

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
@marcoscannabrava @cclauss @mateuszz0000
You can’t perform that action at this time.