You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@groovy.apache.org by Paul King <pa...@asert.com.au> on 2018/09/04 08:17:15 UTC

Proposed @NamedVariant breaking change edge case

Hi folks,

I have been thinking about:

https://issues.apache.org/jira/browse/GROOVY-8703

If you remember the design behind @NamedVariant, there were
two schools of thought. One group were keen on always having the first
parameter as a delegate; it's properties determining the keys for named
args.
Another group were keen on the names of the params themselves
being the keys for named args. Our current design using @NamedParam
and @NamedDelegate market annotations covers both these use cases
and even allows mix and match.

I don't plan on changing anything in cases where either @NamedParam
or @NamedDelegate are used, however, I think I made the wrong call for
what should happen if no marker annotations are placed on the parameters.
I went with defaulting the behavior as if @NamedDelegate was implicitly
added to the first param. In fact, I think it should have been as if
@NamedParam was added to all parameters.

Basically, I propose making this (using the GROOVY-8703 example):

 @NamedVariant
  Color(Integer r, Integer g, Integer b) {
    this.r = r; this.g = g; this.b = b
  }


Be the same as this:

 @NamedVariant
  Color(@NamedParam Integer r, @NamedParam Integer g, @NamedParam Integer b) {

    this.r = r; this.g = g; this.b = b
  }


Given 2.5 has only been out for a few months, I suspect @NamedVariant
uptake won't be huge at this point and most probably folks aren't relying on
the behavior when the additional marker annotations are left off.
I propose we change this behavior and list it as a breaking change in
the 2.5.3 release notes. If we delay any longer though, I think we will need
to stick with the current design.

We could add an autoDelegate annotation attribute to allow the old behavior
to be switched on but I think in fact using @NamedDelegate on the first
parameter is actually clearer anyway. I guess for annotation collector
scenarios
something like autoDelegate might be useful.

Thoughts?

Cheers, Paul.

Re: Proposed @NamedVariant breaking change edge case

Posted by Paul King <pa...@asert.com.au>.
On Wed, Sep 5, 2018 at 2:56 AM Jochen Theodorou <bl...@gmx.org> wrote:

>
>
> Am 04.09.2018 um 10:17 schrieb Paul King:
> [...]
> > Basically, I propose making this (using the GROOVY-8703 example):
> >
> >   @NamedVariant
> >    Color(Integer  r,Integer  g,Integer  b) {
> >      this.r = r;this.g = g;this.b = b
> >    }
> >
> >
> > Be the same as this:
> >
> >   @NamedVariant
> >    Color(@NamedParam Integer r, @NamedParam Integer g, @NamedParam
> Integer b) {
> >
> >      this.r = r;this.g = g;this.b = b
> >    }
>
> reminds me a lot of the code I just saw for the definition of a rest
> endpoint in spring with annotations for about everything.
>
> Can't the usage of @NamedParam imply @NamedVariant?
>

You could do this but currently @NamedParam and @NamedDelegate are marker
interfaces so you'd need a global transform.


> > Given 2.5 has only been out for a few months, I suspect @NamedVariant
> > uptake won't be huge at this point and most probably folks aren't
> relying on
> > the behavior when the additional marker annotations are left off.
> > I propose we change this behavior and list it as a breaking change in
> > the 2.5.3 release notes. If we delay any longer though, I think we will
> need
> > to stick with the current design.
>
> I think we should work more with marking new features as experimental
>

That would have been a great option (in hindsight) here. :-)


>
>   bye Jochen
>

Re: Proposed @NamedVariant breaking change edge case

Posted by Jochen Theodorou <bl...@gmx.org>.

Am 04.09.2018 um 10:17 schrieb Paul King:
[...]
> Basically, I propose making this (using the GROOVY-8703 example):
> 
>   @NamedVariant
>    Color(Integer  r,Integer  g,Integer  b) {
>      this.r = r;this.g = g;this.b = b
>    }
> 
> 
> Be the same as this:
> 
>   @NamedVariant
>    Color(@NamedParam Integer r, @NamedParam Integer g, @NamedParam Integer b) {
> 
>      this.r = r;this.g = g;this.b = b
>    }

reminds me a lot of the code I just saw for the definition of a rest 
endpoint in spring with annotations for about everything.

Can't the usage of @NamedParam imply @NamedVariant?

> Given 2.5 has only been out for a few months, I suspect @NamedVariant
> uptake won't be huge at this point and most probably folks aren't relying on
> the behavior when the additional marker annotations are left off.
> I propose we change this behavior and list it as a breaking change in
> the 2.5.3 release notes. If we delay any longer though, I think we will need
> to stick with the current design.

I think we should work more with marking new features as experimental

  bye Jochen

Re: Proposed @NamedVariant breaking change edge case

Posted by mg <mg...@arscreat.com>.
I think you are right, and having only @NamedVariant on a method should equal also having @NamedParam on every parameter. 
Rationale: I feel it is the more common case & least surprise, adding @NamedParam to all arguments is cumbersome and creates a lot of clutter/noise, and (as you said) making the @NamedDelegate case explicit is a good idea.
+1 on changing this.


-------- Ursprüngliche Nachricht --------Von: Paul King <pa...@asert.com.au> Datum: 04.09.18  10:17  (GMT+01:00) An: dev@groovy.apache.org Betreff: Proposed @NamedVariant breaking change edge case 

Hi folks,
I have been thinking about:
https://issues.apache.org/jira/browse/GROOVY-8703

If you remember the design behind @NamedVariant, there weretwo schools of thought. One group were keen on always having the firstparameter as a delegate; it's properties determining the keys for named args.Another group were keen on the names of the params themselvesbeing the keys for named args. Our current design using @NamedParamand @NamedDelegate market annotations covers both these use casesand even allows mix and match.
I don't plan on changing anything in cases where either @NamedParamor @NamedDelegate are used, however, I think I made the wrong call forwhat should happen if no marker annotations are placed on the parameters.I went with defaulting the behavior as if @NamedDelegate was implicitlyadded to the first param. In fact, I think it should have been as if@NamedParam was added to all parameters.
Basically, I propose making this (using the GROOVY-8703 example):
 @NamedVariant
  Color(Integer r, Integer g, Integer b) {
    this.r = r; this.g = g; this.b = b
  }
Be the same as this:
 @NamedVariant
  Color(@NamedParam Integer r, @NamedParam Integer g, @NamedParam Integer b) {    this.r = r; this.g = g; this.b = b
  }
Given 2.5 has only been out for a few months, I suspect @NamedVariantuptake won't be huge at this point and most probably folks aren't relying onthe behavior when the additional marker annotations are left off.I propose we change this behavior and list it as a breaking change inthe 2.5.3 release notes. If we delay any longer though, I think we will needto stick with the current design.
We could add an autoDelegate annotation attribute to allow the old behaviorto be switched on but I think in fact using @NamedDelegate on the firstparameter is actually clearer anyway. I guess for annotation collector scenariossomething like autoDelegate might be useful.
Thoughts?
Cheers, Paul.