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

GROOVY-9848: in operator: key set membership for isCase(Map,Object) #1904

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

eric-milles
Copy link
Member

@eric-milles
Copy link
Member Author

I added tests for GROOVY-2456 as well. This is probably a good time to revisit that item and decide if isCase should be left alone for maps and strings and some other implementation provided for the in operator.

* <pre class="groovyTestCase">switch( 'foo' ) {
* case [foo:true, bar:false]:
* case [foo:true]:
Copy link
Contributor

Choose a reason for hiding this comment

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

To make it really clear I would go one step further and change [foo:true] to [foo:false]

assert 'foo' in [foo:true]
assert 'foo' in [foo:null]
assert 'bar' !in [foo:true]
assert 'bar' !in [:].withDefault{ true }
Copy link
Contributor

Choose a reason for hiding this comment

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

Again here I would use false as default to be more clear about the keys being used

// GROOVY-7919
void testInForIterables() {
Iterable iter = { -> ['a','b','c'].iterator() }
assert 'a' in iter
Copy link
Contributor

Choose a reason for hiding this comment

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

actually it would be nice to do assert 'a' in iter twice to show that the second time it fails

@paulk-asert
Copy link
Contributor

I think your idea about leaving isCase alone and changing "in" to use "contains" has merit and is worth exploring as an alternative to this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants