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

Add Set#=== as alias to Set#includes? #5269

Merged

Conversation

makenowjust
Copy link
Contributor

This method is for convenience with using on case statement.

Ruby 2.5 decides such a change, this PR follows it. However IMO, this is more natural than current and useful.

This method is for convenience with using on `case` statement.

Ruby 2.5 decides such a change, this commit follows it:
https://bugs.ruby-lang.org/issues/13801
@asterite
Copy link
Member

asterite commented Nov 9, 2017

Please no, I'm against that change. It means this doesn't work:

set1 = Set{1, 2, 3}
set2 = Set{3, 2, 1}

case set1
when set2
  puts "Works!"
else
  puts "Doesn't work!"
end

With this PR it will print "Doesn't work!", totally unpredictable.

There's also the thing that I never, ever have seen a set being used in a when condition. And doing includes? is much more explicit and understandable.

@makenowjust
Copy link
Contributor Author

makenowjust commented Nov 9, 2017

@asterite print "Works!" due to def ===(object : T).

@asterite
Copy link
Member

asterite commented Nov 9, 2017

Ooooooooh... I didn't see that. Yes, that's better than the Ruby version. So maybe it's good :-)

@asterite
Copy link
Member

asterite commented Nov 9, 2017

But then, should Array also behave like this? What makes Set so special that it deserves a ===?

@makenowjust
Copy link
Contributor Author

IMO, ordered (indexed) collection should not be redefine === and unordered collection should be redefine ===. Because ordered collection's item is characterized by key-value pair, not value only.

And another thought: Why Set#=== is not redefined, though Range#=== is redefined? Range is pioneer which is redefine === as includes?.

@makenowjust
Copy link
Contributor Author

ping.

@bcardiff
Copy link
Member

@makenowjust Could some docs like in Range#=== be added so the use case is shown in code?

@makenowjust
Copy link
Contributor Author

@bcardiff Added.

@bcardiff bcardiff added this to the Next milestone Nov 27, 2017
@bcardiff bcardiff merged commit 6ee1bf9 into crystal-lang:master Nov 27, 2017
@bcardiff
Copy link
Member

Thanks @makenowjust !

@makenowjust makenowjust deleted the feature/hash/includes-eqeqeq branch November 27, 2017 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants