-
Notifications
You must be signed in to change notification settings - Fork 15
WIP Add HashSet which uses Open-Addressing to resolve collisions #21
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
WIP Add HashSet which uses Open-Addressing to resolve collisions #21
Conversation
…der in preparation for open addressing implementation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved for merging since its functional but do consider some suggestions for future iterations!
* @param element the element to look for. | ||
* @return the index of the bucket containing the element. | ||
*/ | ||
private int search(T element) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it possible to call your LinearProbe here? i understand your LinearProbe now does additional stuff - maybe can isolate each functionality as per SRP (single-responsibility principle)
} | ||
|
||
// This method returns an instance of Tombstone, which is used to mark removed elements. | ||
private T tombstone() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah okay. Tombstone can be seen as an object by itself. I was initially confused because i thought this was tracked somewhere as an attribute. Perhaps Tombstone can be a private class and serve as a static (correct me if im wrong, but i instance of it should suffice right? ultimately its a placeholder) attribute of this HashSet class?
} | ||
currentBucketIndex = (currentBucketIndex + 1) % this.capacity(); | ||
} | ||
resize(); // TODO implement resize operation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup, to shift this away
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job! do lmk before merging
*/ | ||
@Override | ||
public int hashCode() { | ||
return System.identityHashCode(this); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh wait whats the difference between simply calling hashCode method of the object ah?
} | ||
currentBucketIndex = (currentBucketIndex + 1) % this.capacity(); | ||
} | ||
return ELEMENT_NOT_FOUND; // placeholder return value for now. Will never reach this line. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okay yeah its strange that it returns this. If you are sure this line will never be reached, you can just return null and even do assert False here (with a message that thats it should not be reached/happen)
// Checks equality of element using Object::equals and Object::hashCode | ||
if (element.equals(existingElement) | ||
&& element.hashCode() == existingElement.hashCode()) { | ||
return currentBucketIndex; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually is this a design decision? if u have this, do you still need an explicit search method? or does the search method exists for completeness?
When elements are hashed to the same initial bucket, and then an element is removed, there is a possibility that a duplicate element can be inserted into the place of the deleted element.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Implemented using linear probe to probe for empty buckets.
Currently incomplete implementation and has fixed size/number of buckets.