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

Alias #key? as #has_key? #57

Closed
wants to merge 1 commit into from
Closed

Alias #key? as #has_key? #57

wants to merge 1 commit into from

Conversation

endash
Copy link
Contributor

@endash endash commented Feb 28, 2019

It would be nice if #key? were aliased to #has_key? so that the expect(container).to have_key(key) RSpec matcher can be used instead of expect(container.key?(key)).to be(true)

@flash-gordon
Copy link
Member

I wouldn't do it, I like Container's interface being close to Hash but the status of Hash#has_key? isn't clear http://blade.nagaokaut.ac.jp/cgi-bin/scat.rb/ruby/ruby-core/43765

@endash
Copy link
Contributor Author

endash commented Feb 28, 2019

Interesting. I actually was surprised to find that the have_key matcher didn't just use key?, so it definitely seems to be one of those hidden dusty corners of Ruby. 7 years on, though, it doesn't seem like has_key? is going anywhere, and in the meantime it'd be nice to test using the same matchers.

I suppose I could take it up with rspec-expectations first, but I suspect they'd be disinclined to change something so battle-tested, in order to accommodate classes that partially duck-subtype Hash.

It's a bit of a catch-22... rspec likely don't see any reason to change what works, but on the other hand perpetuating an API that Matz considers a design mistake is less than ideal.

Won't hurt to ask there, though. I can't think of a practical reason why just changing the matcher to #key? wouldn't be a universally acceptable solution, other than caution.

…key RSpec matcher can be used instead of key?(key) eq(true)
@endash
Copy link
Contributor Author

endash commented Feb 28, 2019

Sigh. Never mind. This is a dynamic matcher, playing off of the has prefix. That makes it a "user's choice" API decision, out of their hands.

# You can use this feature to invoke any predicate that begins with "has_", whether it is
# part of the Ruby libraries (like `Hash#has_key?`) or a method you wrote on your own class.

Maybe including a custom matcher a user can include in their spec_helper... but at that point they might as well just implement it themselves, trivially:

RSpec::Matchers.define(:have_key) do |expected|
  match { |actual| actual.key?(expected) }
end

Personally, I don't see a problem with just aping Hash all that more closely... but I see the other argument, too.

@flash-gordon
Copy link
Member

I would go with a matcher since the only "real" reason for having it is good looking specs. And anyway, if we add has_key? why not include? etc.
We can add the matcher to source code, not a problem

# load it explicitly
require 'dry/container/rspec/matcher'

# and here you go
is_expected.to have_key(key)

@endash
Copy link
Contributor Author

endash commented Feb 28, 2019

Discovered you can also do

is_expected.to be_key(key)

which will invoke the ? predicate matcher. If you want to be both terse AND semantically inaccurate.

I think you're right though @flash-gordon, it's mildly satisfying to have the cleaner looking expect, which is probably worth tossing in an optional require, but if the intention is not to replicate the Hash interface, and it probably shouldn't be, then best to let sleeping dogs lie in this case.

@endash endash closed this Mar 5, 2019
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.

2 participants