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.

Sign up for GitHub

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

Jump to bottom

Improve value_object's #hash uniformity #233

Merged
merged 1 commit into from Dec 19, 2013
Merged

Improve value_object's #hash uniformity #233

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/virtus/support/equalizer.rb
View file
Open in desktop
Expand Up @@ -65,7 +65,7 @@ def define_cmp_method
def define_hash_method
keys = @keys
define_method(:hash) do
keys.map { |key| send(key).hash }.reduce(self.class.hash, :^)
keys.map { |key| send(key) }.push(self.class).hash
Copy link
Collaborator

Choose a reason for hiding this comment

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

In similar code, I've done something like the following because I wanted the hash keys to be factored into the hash:

keys.map { |key| key.hash ^ send(key).hash }.reduce(self.class.hash, :^)

I like your solution though because the work to create the Array has already been done anyway. Might as well call Array#hash rather than using Enumerable#reduce to xor the hashes.. it's simpler and probably optimized.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if you want your keys counted in #hash, you should probably just use
Hash#hash. It's uniform and calls #hash on all the keys and values
recursively.

using just xor is bad for uniformity. Though, that matters only if you use
your objects as Hash keys or in sets or if you do .uniq on them. You don't
want 1000's of your objects in one bucket.

On Thu, Dec 19, 2013 at 10:34 AM, Dan Kubb notifications@github.com wrote:

In lib/virtus/support/equalizer.rb:

@@ -65,7 +65,7 @@ def define_cmp_method
def define_hash_method
keys = @keys
define_method(:hash) do

  •    keys.map { |key| send(key).hash }.reduce(self.class.hash, :^)

  •    keys.map { |key| send(key) }.push(self.class).hash

In similar code, I've done something like the following because I wanted
the hash keys to be factored into the hash:

keys.map { |key| key.hash ^ send(key).hash }.reduce(self.class.hash, :^)

I like your solution though because the work to create the Array has
already been done anyway. Might as well call Array#hash rather than using
Enumerable#reduce to xor the hashes.. it's simpler and probably optimized.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/233/files#r8461740
.

end
end

Expand Down