-
-
Notifications
You must be signed in to change notification settings - Fork 388
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
Extend specs of Set#divide #1049
Conversation
The description in the docs state "Returns an enumerator if no block is given", added in Ruby 2.4.
library/set/divide_spec.rb
Outdated
it "only uses the first element if the arity is not 2" do | ||
set = Set["one", "two", "three", "four", "five"].divide { |x, _, _| x.length } | ||
set.map { |x| x.to_a.sort }.sort.should == [["five", "four"], ["one", "two"], ["three"]] | ||
end |
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.
suggestion: I would move out this case into a separate describe
/`context' (with a title like "when passed a block with an arity > 2") for consistency.
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.
suggestion: Also I like the check in the case above:
ret.each(&:even?).should == Set[Set[1, 3], Set[2, 4]]
using Set
instead of sorting looks more readable and shorted
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.
suggestion: TBH I would find a case title a bit laconic and not comprehensive (as well as in a similar case below for arity = 2). It will be great if the titles reflect documentation and describe exactly what happens and how a set elements are compared/divided:
If the arity of the block is 2, elements o1 and o2 are in common if block.call(o1, o2) is true. Otherwise, elements o1 and o2 are in common if block.call(o1) == block.call(o2).
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.
suggestion: I would move out this case into a separate
describe
/`context' (with a title like "when passed a block with an arity > 2") for consistency.
I extracted this one into its own block, and added a block with a splat argument as well (which also should use the first element, since this results in an arity of -1
, this might be a nice extra test case for Ruby implementations)
I think the other two entries are nice additions, but slightly out of scope for this case.
This is undocumented behaviour.
Thank you for the specs! |
This contains two commits:
Set#divide
without a block. There was a commented piece of code that expected an exception, but since Ruby 2.4 the documentation states it should return an Enumerator