Add Hash#key? vs. Hash#keys.include? and Hash#value? vs. Hash#values.include? - #154

Thanks for the contribution. But is this something that people do in practice? I don't think I've ever encountered someone calling keys.include?. It's usually a matter of Hash#key? vs Hash#[].

https://github.com/search?utf8=%E2%9C%93&q=%22keys%5C.include%5C%3F%22+extension%3Arb+language%3ARuby&type=Code&ref=advsearch&l=Ruby&l=

Our team is a group of pretty experienced Ruby developers with a strict code review policy, and I noticed both of these patterns appear in our codebases. I suspect we're not alone.

I actually made this PR because I caught myself writing values.include? which isn't obviously slower than value?, which is why I looked at the Ruby implementation and then tested it to confirm my suspicion that it was.

There sure are a lot of ips blocks there.
I get the whole hit and miss use cases. (I did this recently while testing) - I just wonder if this is overkill for this project?

Also, I'm amused that Hash[key] vs Hash.key?(key) is not there. That is how I usually check for presence.

I too am surprised people use Hash.keys.include?(key) - so many allocations and all.

Can you please split that into 2 sections: one for keys and the other for values - IMO it will look much more readable.

@ixti thanks for the suggestion; done.

I also added tests for !!Hash[key] as @kbrock suggested, though I go back and forth on whether the !! should be in the test or not. I'm also not totally sure that Hash[key] should be in here at all, since it's not really a presence check—if the value is falsey the presence check will fail.

Thoughts?

I recommend avoid having Hash#[](key) test in here to avoid confusion. In some cases, Hash#[] is fine for such tests like:

return unless RANGES[key]

But it may lead to confusion:

data = { :foo => nil, :bar => false, :baz => 1 }

%i[foo bar baz].map { |k| data.key? k } # => [true, true, true]
%i[foo bar baz].map { |k| data[k] ? true : false } # [false, false, true]

And after all Hash#[] returns value, so !!data[k] testing value truthiness rather than presence of key. :D

Oof, you're right @ixti—can't believe I missed that !! issue. I've removed Hash#[] from this PR.

Thank you!