Skip to content
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

Hash eq board #45

Merged
merged 5 commits into from
Mar 12, 2021
Merged

Hash eq board #45

merged 5 commits into from
Mar 12, 2021

Conversation

Nicholas-Baron
Copy link
Contributor

Solves #44

This PR makes Board possible to use as the key of a standard Rust HashMap.
Most of the changes are additions to #[derive] attribute for the types.

Of note is the impl Hash for Board. The hash function does not return its result and instead has a &mut Hasher parameter to hold the state of the hash. The current solution is to only hash self.hash. This is faster than the #[derive], as it is only one field, but I do not know the implications of using a hash of a hash.

@Nicholas-Baron Nicholas-Baron marked this pull request as ready for review March 2, 2021 04:39
@jordanbray
Copy link
Owner

I'll look over this tonight I think. Nothing looks crazy to me off hand. There's no reason to believe hashing the hash is going to be a problem, other than a performance hit. I do wish there was a better way, but I don't know of one off-hand.

@Nicholas-Baron
Copy link
Contributor Author

@jordanbray Are there any comments from your review that I should implement?

@jordanbray
Copy link
Owner

Sorry for the delay. This all looks good to me. I ran some performance tests just to be sure nothing funky happened, and it didn't.

@jordanbray jordanbray merged commit 36bfda0 into jordanbray:master Mar 12, 2021
@Nicholas-Baron Nicholas-Baron deleted the hash_eq_board branch March 12, 2021 04:52
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.

None yet

2 participants