[jira] [Commented] (CALCITE-4254) ImmutableBeans should make an immutable copy of property values of type List, Set, or Map

2020-09-14 Thread Vladimir Sitnikov (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-4254?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17195637#comment-17195637
 ] 

Vladimir Sitnikov commented on CALCITE-4254:


Julian, what do you think ofrenaming {{makeImmutable}} attribute to 
{{mutable}}, so the use becomes {{makeMutable=true}} rather than 
{{immutable=false}}?

Just in case, another idea might be Java 1.8 type annotations, so the 
declaration might be like

{code:java}
@ImmutableBean.Property
@Mutable List getNumbers();
{code}

A nice consequence would be the same design might work for cases like 
{code:java}Map>{code}, and, more important, 
{{@Mutable}} annotations would be visible in the IDE, so users see that the 
list is mutable or not.


> ImmutableBeans should make an immutable copy of property values of type List, 
> Set, or Map
> -
>
> Key: CALCITE-4254
> URL: https://issues.apache.org/jira/browse/CALCITE-4254
> Project: Calcite
>  Issue Type: Bug
>Reporter: Julian Hyde
>Assignee: Julian Hyde
>Priority: Major
>
> {{ImmutableBeans}} should make an immutable copy of values of type {{List}}, 
> {{Set}} or {{Map}} when they are provided via a setter method. For example, 
> this is how it should work:
> {code:java}
> MyBean bean = ImmutableBeans.create(MyBean.class);
> List evens = Arrays.asList(2, 4, 6);
> bean = bean.withNumbers(evens);
> assertThat(bean.getNumbers(), equalTo(evens));
> assertThat(bean.getNumbers(), isInstanceOf(ImmutableList.class));
> {code}
> Note that {{evens}} has been converted to an {{ImmutableList}} with the same 
> contents.
> You can control this behavior by setting {{makeImmutable = false}} when you 
> declare the property:
> {code:java}
> interface MyBean {
>   @ImmutableBean.Property(makeImmutable = false)
>   List getNumbers();
>   MyBean withNumbers(List numbers);
> }
> {code}
> By default, {{makeImmutable}} is true. This will change behavior of a very 
> few existing beans that were assuming that the value that was put in would be 
> the value that came out (see a couple of changes in {{VolcanoPlannerTest}}), 
> but for the vast majority, immutability is the desired behavior.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (CALCITE-4254) ImmutableBeans should make an immutable copy of property values of type List, Set, or Map

2020-09-14 Thread Vladimir Sitnikov (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-4254?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17195608#comment-17195608
 ] 

Vladimir Sitnikov commented on CALCITE-4254:


Oh, I just re-read the issue description, and now I see it suggests "immutable 
by default, and add escape hatch for a mutable case". That is awesome for me.

> ImmutableBeans should make an immutable copy of property values of type List, 
> Set, or Map
> -
>
> Key: CALCITE-4254
> URL: https://issues.apache.org/jira/browse/CALCITE-4254
> Project: Calcite
>  Issue Type: Bug
>Reporter: Julian Hyde
>Assignee: Julian Hyde
>Priority: Major
>
> {{ImmutableBeans}} should make an immutable copy of values of type {{List}}, 
> {{Set}} or {{Map}} when they are provided via a setter method. For example, 
> this is how it should work:
> {code:java}
> MyBean bean = ImmutableBeans.create(MyBean.class);
> List evens = Arrays.asList(2, 4, 6);
> bean = bean.withNumbers(evens);
> assertThat(bean.getNumbers(), equalTo(evens));
> assertThat(bean.getNumbers(), isInstanceOf(ImmutableList.class));
> {code}
> Note that {{evens}} has been converted to an {{ImmutableList}} with the same 
> contents.
> You can control this behavior by setting {{makeImmutable = false}} when you 
> declare the property:
> {code:java}
> interface MyBean {
>   @ImmutableBean.Property(makeImmutable = false)
>   List getNumbers();
>   MyBean withNumbers(List numbers);
> }
> {code}
> By default, {{makeImmutable}} is true. This will change behavior of a very 
> few existing beans that were assuming that the value that was put in would be 
> the value that came out (see a couple of changes in {{VolcanoPlannerTest}}), 
> but for the vast majority, immutability is the desired behavior.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (CALCITE-4254) ImmutableBeans should make an immutable copy of property values of type List, Set, or Map

2020-09-14 Thread Vladimir Sitnikov (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-4254?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17195607#comment-17195607
 ] 

Vladimir Sitnikov commented on CALCITE-4254:


Well, there's one more option: use immutable by default, and provide a way to 
unlock mutable when it is needed.

> ImmutableBeans should make an immutable copy of property values of type List, 
> Set, or Map
> -
>
> Key: CALCITE-4254
> URL: https://issues.apache.org/jira/browse/CALCITE-4254
> Project: Calcite
>  Issue Type: Bug
>Reporter: Julian Hyde
>Assignee: Julian Hyde
>Priority: Major
>
> {{ImmutableBeans}} should make an immutable copy of values of type {{List}}, 
> {{Set}} or {{Map}} when they are provided via a setter method. For example, 
> this is how it should work:
> {code:java}
> MyBean bean = ImmutableBeans.create(MyBean.class);
> List evens = Arrays.asList(2, 4, 6);
> bean = bean.withNumbers(evens);
> assertThat(bean.getNumbers(), equalTo(evens));
> assertThat(bean.getNumbers(), isInstanceOf(ImmutableList.class));
> {code}
> Note that {{evens}} has been converted to an {{ImmutableList}} with the same 
> contents.
> You can control this behavior by setting {{makeImmutable = false}} when you 
> declare the property:
> {code:java}
> interface MyBean {
>   @ImmutableBean.Property(makeImmutable = false)
>   List getNumbers();
>   MyBean withNumbers(List numbers);
> }
> {code}
> By default, {{makeImmutable}} is true. This will change behavior of a very 
> few existing beans that were assuming that the value that was put in would be 
> the value that came out (see a couple of changes in {{VolcanoPlannerTest}}), 
> but for the vast majority, immutability is the desired behavior.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (CALCITE-4254) ImmutableBeans should make an immutable copy of property values of type List, Set, or Map

2020-09-14 Thread Julian Hyde (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-4254?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17195601#comment-17195601
 ] 

Julian Hyde commented on CALCITE-4254:
--

[~vlsi], Ideally we'd allow both ways. In your scheme, the wither method would 
have type {{List}} and the getter method would have type {{ImmutableList}}. In 
my scheme, the wither and getter would both have type {{List}}.

My scheme would be preferred by people who prefer to keep Guava types out of 
interfaces. Plus, the implementation currently does not allow the property to 
have different types in wither and getter.

> ImmutableBeans should make an immutable copy of property values of type List, 
> Set, or Map
> -
>
> Key: CALCITE-4254
> URL: https://issues.apache.org/jira/browse/CALCITE-4254
> Project: Calcite
>  Issue Type: Bug
>Reporter: Julian Hyde
>Assignee: Julian Hyde
>Priority: Major
>
> {{ImmutableBeans}} should make an immutable copy of values of type {{List}}, 
> {{Set}} or {{Map}} when they are provided via a setter method. For example, 
> this is how it should work:
> {code:java}
> MyBean bean = ImmutableBeans.create(MyBean.class);
> List evens = Arrays.asList(2, 4, 6);
> bean = bean.withNumbers(evens);
> assertThat(bean.getNumbers(), equalTo(evens));
> assertThat(bean.getNumbers(), isInstanceOf(ImmutableList.class));
> {code}
> Note that {{evens}} has been converted to an {{ImmutableList}} with the same 
> contents.
> You can control this behavior by setting {{makeImmutable = false}} when you 
> declare the property:
> {code:java}
> interface MyBean {
>   @ImmutableBean.Property(makeImmutable = false)
>   List getNumbers();
>   MyBean withNumbers(List numbers);
> }
> {code}
> By default, {{makeImmutable}} is true. This will change behavior of a very 
> few existing beans that were assuming that the value that was put in would be 
> the value that came out (see a couple of changes in {{VolcanoPlannerTest}}), 
> but for the vast majority, immutability is the desired behavior.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)