You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@groovy.apache.org by "Eric Milles (Jira)" <ji...@apache.org> on 2023/04/23 21:58:00 UTC

[jira] [Comment Edited] (GROOVY-6144) Confusing treatment of m['foo'], m.get('foo'), m.foo, m.getFoo() for Maps

    [ https://issues.apache.org/jira/browse/GROOVY-6144?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17482026#comment-17482026 ] 

Eric Milles edited comment on GROOVY-6144 at 4/23/23 9:57 PM:
--------------------------------------------------------------

https://github.com/apache/groovy/commit/051d3cf3ef29a521137b9d6b71cb8097d077d46b
https://github.com/apache/groovy/commit/bbbff15b867157d8ed9f05207265a5ea80357ba6


was (Author: emilles):
https://github.com/apache/groovy/commit/051d3cf3ef29a521137b9d6b71cb8097d077d46b

> Confusing treatment of m['foo'], m.get('foo'), m.foo, m.getFoo() for Maps
> -------------------------------------------------------------------------
>
>                 Key: GROOVY-6144
>                 URL: https://issues.apache.org/jira/browse/GROOVY-6144
>             Project: Groovy
>          Issue Type: Bug
>          Components: groovy-runtime
>            Reporter: Richard Rattigan
>            Assignee: Eric Milles
>            Priority: Major
>              Labels: breaking
>             Fix For: 5.0.0-alpha-1
>
>
> See the following example for some Map-related quirks that have bitten me a few times. I think it's crucial to iron out these inconsistencies in order to prevent the grooviness of maps from being a liability.
> I think the rules should be:
> 1. subscript operator always uses map interface (for both get and set/put)
> 2. property access always prefers setFoo(bar) over set('foo', bar)
> 3. existence of setFoo() or getFoo() prevents use of map interface for property access, whether reading or writing (to avoid inconsistency between x = m.foo and m.foo = x)
> I guess that accessing m.'foo' should be equivalent to accessing m.foo, but maybe it should be equivalent to m['foo']. I can see arguments for both.
> {code}
> def m = [:]
> // 1. as expected, accessing a map by subscript
> assert m['class'] == null
> assert m['metaClass'] == null
> // 2. which of these should pass? I think the current behaviour is questionable:
> // getters should be preferred over map members for this syntax.
> // I would expect these to pass instead: 
> // assert m.class == m.getClass()
> // assert m.metaClass == m.getMetaClass()
> assert m.class == null // WRONG
> assert m.'class' == null // WRONG (?)
> assert m.metaClass == null // WRONG
> assert m.'metaClass' == null // WRONG (?)
>  
> // 3. as expected, using map interface method
> assert m.get('class') == null
> // 4. as expected, using getter
> assert m.getClass() instanceof Class
> // 5. as expected, accessing map by subscript
> assert (m['class'] = 'foo') == 'foo'
> // 6. this is unexpected, and it is inconsistent with (1) and (5). I would expect the following to pass, 
> // since we're using the subscript operator:
> // assert (m['metaClass'] = 'foo') == 'foo'
> try { m['metaClass'] = 'foo'; assert false } catch (ClassCastException e) { } // WRONG
> // 7. this is consistent with (2), but I would argue that this should fail by the same logic: 
> // Maps have a getClass(), therefore it has a property 'class'. 
> // Attempting to set the property should attempt to invoke setClass(), which doesn't exist.
> // It should therefore throw a MissingMethodException for setClass()
> assert (m.class = 'foo') == 'foo' // WRONG
> assert (m.'class' = 'foo') == 'foo' // WRONG (?)
> // 8. I think this is doing the right thing - preferring the property over map semantics - but it is inconsistent with (2)
> try { m.metaClass = 'foo'; assert false } catch (ClassCastException e) { }
> try { m.'metaClass' = 'foo'; assert false } catch (ClassCastException e) { }
> // 9. as expected, looking for setter, none exists
> try { m.setClass('foo'); assert false } catch (MissingMethodException e) { }
> {code}



--
This message was sent by Atlassian Jira
(v8.20.10#820010)