groovy-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From MG <mg...@arscreat.com>
Subject Re: Making @Immutable a meta-annotation
Date Sat, 20 Jan 2018 00:03:26 GMT
@MapConstructor(makeImmutable=true)  (or maybe: 
@Constructor(defensiveCopying=true, cloneNonImmutableObjects = false,  
/*etc*/) ?) for me would be the way to go.

(No implementation detail smell imho, just fine granularity for 
developers*, and not reusing the same annotation for (slightly) 
different things.)

Cheers,
mg

*Which is always good in my book, plus most people will use @Immutable 
meta-annotation anyway, plus everyone can create their own 
meta-annotations from these fine granular annotations (to avoid code 
clutter)


On 17.01.2018 01:48, Paul King wrote:
>
> Response inline.
>
> On Wed, Jan 17, 2018 at 9:54 AM, MG <mgbiz@arscreat.com 
> <mailto:mgbiz@arscreat.com>> wrote:
>
>     Hmmm.... If the argument for naming the marker annotation
>     @KnownImmutable was that the existing parameters have similar
>     names (and cannot be changed) then it seems to me the
>     "KnownImmutable" name choice was pretty immutable to begin with...
>
>     Apart from that, there is still the inconsistency what
>     @KnownImmutable  really expresses:
>
>       * Class that carries @KnownImmutable is "fully immutable": When
>         a developer puts the annotation on a class
>       * Class that carries @KnownImmutable is "base immutable" (i.e.
>         no defense-copying ctors etc): When being put by the Groovy
>         compiler on a class after having applied @ImmutableBase
>         transformations to it.
>
> This second bullet is the wrong way to look at what is going on. Just 
> using @ImmutableBase (or whatever name) on a class doesn't add the 
> @KnownImmutable annotation. The @Immutable annotation collector adds 
> @KnownImmutable knowing that @ImmutableBase and the various 
> constructor annotations are going to be processed. So in fact it's 
> just the first case but with the compiler indicating that it is "fully 
> immutable". So I don't see a conflict as far as the @Immutable 
> annotation goes. What you could argue is that users of the constructor 
> transformations might want the defensive copying etc. but might not 
> want to make the class fully immutable in which case having an 
> annotation attribute like @MapConstructor(makeImmutable=true) would 
> make more sense. This would provide maximum flexibility and remove the 
> slight dual usage of the annotation. But the other way to look at this 
> is that having a "makeImmutable" annotation attribute smells of 
> implementation detail and just using @KnownImmutable is the more 
> declarative way to express what we want to achieve with less noise.
>
>     The way it looks to me you are trying to express two different
>     things through the same annotation - but to have a clean design
>     you would need two seperate annotations. Maybe that is also why
>     you do not like any of my alternatives, because you are looking
>     for one name that expresses both use cases - and that does not
>     exist, because the use cases differ (?)
>
>     I am still convinced that while knownUmmutable semi-works as a
>     parameter name inside of @Immutable (I would have picked
>     guaranteed here also), that does not mean it is a good choice for
>     the annotation name. But as I said, if you are convinced that one
>     requires the other, this discussion is mute anyway...
>
>
>     On 16.01.2018 01:56, Paul King wrote:
>>     Explanations below.
>>
>>     On Tue, Jan 16, 2018 at 12:56 AM, MG <mgbiz@arscreat.com
>>     <mailto:mgbiz@arscreat.com>> wrote:
>>
>>         Hi Paul,
>>
>>         1) for me, if you have to explain a name better, then it is
>>         already a bad name. Intuitively suggesting the correct
>>         interpretation to another developer, without requiring him to
>>         thoroughly read through the documentation, is the art of
>>         picking good names (which imho Groovy overall does a good job
>>         at).
>>         With regards to @KnownImmutable, "someone (the compiler or
>>         the developer) knows" is even more confusing. If it is in
>>         fact irrelevant who knows it, why is there a "Known" in the
>>         name in the first place ? And why is therefore e.g.
>>         @IsImmutable not a better name (it could also carry a
>>         parameter which can be true or false, with false allowing a
>>         developer to express that a class is definitely not immutable
>>         (even if it might look that way on first glance; e.g.
>>         effectively blocking or issuing a warning in certain parallel
>>         execution scenarios)).
>>
>>
>>     We have since the introduction of @Immutable used the
>>     knownImmutable and knownImmutableClasses annotation attributes
>>     and people seem to grok what they mean. This is a very similar
>>     use case. I think it would be hard to justify renaming
>>     @KnownImmutable without renaming the annotation attributes as well.
>>
>>         2) There seems to be a contradiction in your statements: You
>>         say that "Once @ImmutableBase (or whatever name) processing
>>         has finished its checks, it can then vouch for the class and
>>         puts the marker interface [@KnownImmutable] "rubber stamp" on
>>         it", but further down you say that "These changes [that
>>         @ImmutableBase applies] alone don't guarantee immutability.".
>>         Is it a "known immutable" after @ImmutableBase has done its
>>         thing, or not ?
>>
>>
>>     Only after all transformations have completed it is guaranteed
>>     (see below).
>>
>>         3) If I did not miss something the new @Immutable meta
>>         annotation is made up of the following annotations:
>>         @ImmutableBase
>>         @KnownImmutable
>>         @ToString
>>         @EqualsAndHashCode
>>         @MapConstructor
>>         @TupleConstructor
>>
>>         How is any of the last four necessary for a class to be
>>         immutable ? Immutability to me means, that the state of the
>>         class cannot be changed after it has been created. How are
>>         @ToString, @EqualsAndHashCode, @MapConstructor, and
>>         @TupleConstructor helping with this ?
>>         At least one ctor to initialize the class fields is basically
>>         necessary to make this a practically usable immutable class,
>>         yes, but @IsImmutable it must be after @ImmutableBase does
>>         its job, or it will not be immutable in the end. All the
>>         other annotations are just icing on the cake (see "@Immutable
>>         should be named @ImmutableCanonical").
>>
>>
>>     @MapConstructor and @TupleConstructor do different things if they
>>     find the @KnownImmutable marker interface on the class they are
>>     processing (defensive copy in/clone/wrap etc.) which is needed
>>     for immutable classes. We could have used an additional
>>     annotation attribute (makeImmutable = true) or something but the
>>     marker interface is useful in its own right and it seems sensible
>>     to not duplicate the information it conveys. Besides we'd have to
>>     choose a name for "makeImmutable" and again since it's only part
>>     of the immutable story good naming would be hard.
>>
>>         If you keep @ImmutableBase, at least consider replacing
>>         @KnownImmutable with @GuaranteedImmutableTag or
>>         @GuaranteedImmutableMarker ? The "Tag" or "Marker" postfix at
>>         least expresses that this annotation just tags the class as
>>         having certain properties, and that this is a general fact,
>>         and not only known to developers or compilers in the know...
>>
>>
>>     Marker interfaces are commonplace within the Java world and we
>>     don't name them as such. It's not CloneableTag or
>>     SerializableMarker. I think adding such a suffix would be confusing.
>>
>>         I hope I do not completely miss your point, but this is how
>>         it looks to me from what I read :-),
>>         Cheers,
>>         mg
>>
>>
>>
>>         On 15.01.2018 14:08, Paul King wrote:
>>>
>>>         Response below.
>>>
>>>         On Sun, Jan 14, 2018 at 6:11 AM, MG <mgbiz@arscreat.com
>>>         <mailto:mgbiz@arscreat.com>> wrote:
>>>
>>>             Hi Paul,
>>>
>>>             now I get where you are coming from with
>>>             @KnownImmutable. I agree with splitting the two
>>>             concepts: Flexible & elegant :-)
>>>
>>>             Transferring the parameter name knownImmutables (which
>>>             exists inside the @Immutable context) to the annotation
>>>             name KnownImmutable (which has no such context) still
>>>             does not work for me, though.
>>>             In addition having @Immutable = @KnownImmutable +
>>>             @ImmutableBase violates the definition you give for
>>>             @KnownImmutable, because either the class is "known to
>>>             be immutable" = "immutable by implementation by the
>>>             developer", or it becomes immutable through
>>>             @ImmutableBase & Groovy...
>>>
>>>
>>>         Well that is perhaps an indication that it needs to be
>>>         explained better rather than necessarily a bad name. I'll
>>>         try again. It just means that someone (the compiler or the
>>>         developer) knows that it is immutable. If that marker
>>>         interface is on the class there is no need to look further
>>>         inside the class, you can assume it is vouched for as
>>>         immutable. Once @ImmutableBase (or whatever name) processing
>>>         has finished its checks, it can then vouch for the class and
>>>         puts the marker interface "rubber stamp" on it.
>>>
>>>             What do you think about:
>>>             @IsImmutable
>>>             @ImmutableContract
>>>             @GuaranteedImmutable
>>>             instead
>>>             ?
>>>
>>>             Thinking about this some more, still don't like
>>>             @ImmutableBase. Sounds too much like a base class to me
>>>             - and what would be the "base" functionality of being
>>>             immutable ? Something either is immutable, or not
>>>             (@ImmutableCore also fails in this regard ;-) ).
>>>             So still would prefer @ImmutableOnly o.s. ..
>>>
>>>
>>>         @ImmutableOnly indicates that it is somehow immutable at
>>>         that point - it isn't really a finished immutable class
>>>         until all the other related transforms have done their
>>>         thing. Perhaps it is useful to reiterate what it does. It
>>>         does a whole pile of validation (you can't have public
>>>         fields, you can't have certain annotation attributes on some
>>>         of the other annotations that wouldn't make sense for an
>>>         immutable object, you can't have your own constructors, it
>>>         can't be applied on interfaces, it checks spelling of
>>>         property names referenced in annotation attributes) plus
>>>         some preliminary changes (makes class final, ensures
>>>         properties have a final private backing field and a getter
>>>         but no setter, makes a copyWith constructor if needed).
>>>         These changes alone don't guarantee immutability. Would you
>>>         prefer @ImmutablePrelim?
>>>
>>>             Cheers,
>>>             mg
>>>
>>>
>>>             -------- Ursprüngliche Nachricht --------
>>>             Von: Paul King <paulk@asert.com.au>
>>>             <mailto:paulk@asert.com.au>
>>>             Datum: 13.01.18 13:17 (GMT+01:00)
>>>             An: MG <mgbiz@arscreat.com> <mailto:mgbiz@arscreat.com>
>>>             Betreff: Re: Making @Immutable a meta-annotation
>>>
>>>             I should have explained the @KnownImmutable idea a bit
>>>             more. I guess I was thinking about several possibilities
>>>             for that in parallel. What I really think is the way to
>>>             go though is to split out the two different aspects that
>>>             I was trying to capture. One is triggering the AST
>>>             transformation, the other is a runtime marker of
>>>             immutability. With that in mind I'd suggest the following:
>>>
>>>             @KnownImmutable will be a marker interface and nothing
>>>             more. Any class having that annotation will be deemed
>>>             immutable.
>>>             E.g. if I write my own Address class and I know it's
>>>             immutable I can mark it as such:
>>>
>>>             @KnownImmutable
>>>             class Address {
>>>             Address(String value) { this.value = value }
>>>               final String value
>>>             }
>>>
>>>             Now if I have:
>>>
>>>             @Immutable
>>>             class Person {
>>>               String name
>>>               Address address
>>>             }
>>>
>>>             Then the processing associated with @Immutable won't
>>>             complain about a potentially mutable "Address" field.
>>>
>>>             Then we can just leave @ImmutableBase (or similar) as
>>>             the AST transform to kick off the initial processing
>>>             needed for immutable classes.
>>>             The @Immutable annotation collector would be replaced by
>>>             the constructor annotations, ToString, EqualsAndHashcode
>>>             and both ImmutableBase and KnownImmutable.
>>>             The name KnownImmutable matches existing functionality.
>>>             Two alternatives to annotating Address with
>>>             KnownImmutable that already exist would be using the
>>>             following annotation attributes on @Immutable:
>>>             @Immutable(knownImmutableClasses=[Address]) or
>>>             @Immutable(knownImmutables=[address]).
>>>
>>>             Cheers, Paul.
>>>
>>>
>>>
>>>             On Sat, Jan 13, 2018 at 1:43 PM, MG <mgbiz@arscreat.com
>>>             <mailto:mgbiz@arscreat.com>> wrote:
>>>
>>>                 Hi Paul,
>>>
>>>                 I think the core of the problem is, that @Immutable
>>>                 as a meta-annotation woud be better off being called
>>>                 something along the line of @ImmutableCanonical
>>>                 (see: If you do no need the immutability, use
>>>                 @Canonical), since it does not solely supply
>>>                 immutability support - then it would be natural to
>>>                 call the actual core immutability annotation just
>>>                 "Immutable".
>>>
>>>                 That is probably off the table, since it would be a
>>>                 breaking change - so we are stuck with the problem
>>>                 of naming the immutability annotation part something
>>>                 else.
>>>
>>>                 @ImmutableClass would imply to me that the "Class"
>>>                 part carries some meaning, which I feel it does not,
>>>                 since
>>>                 a) "Class" could be postfixed to any annotation name
>>>                 that applies to classes
>>>                 b) The meta-annotation should accordingly also be
>>>                 called "ImmutableClass"
>>>                 Because of that I find postfixing "Immutable" with
>>>                 "Class" just confusing. It also is not intuitive to
>>>                 me, which annotation does only supply the core, and
>>>                 which supplies the extended (canonical) functionality...
>>>
>>>                 I do not understand where you are going with
>>>                 @KnownImmutable (known to whom ?-) To me this seems
>>>                 less intuitive/more confusing than @ImmutableClass...).
>>>
>>>                 @ImmutableCore is similar to @ImmutableBase (because
>>>                 I intentionally based it on it :-) ), but different
>>>                 in the sense that it imho expresses the semantics of
>>>                 the annotation: Making the object purely
>>>                 immutable-only, without any constructors, toString
>>>                 functionality, etc.
>>>
>>>                 How about:
>>>                 @ImmutableOnly
>>>                 @PureImmutable
>>>                 @ModificationProtected
>>>
>>>                 @Locked
>>>                 @Frozen
>>>
>>>                 @Unchangeable
>>>                 @Changeless
>>>
>>>                 @InitOnly
>>>                 @InitializeOnly
>>>
>>>                 @Constant
>>>                 @Const
>>>
>>>                 @NonModifieable
>>>                 @NonChangeable
>>>
>>>                 ?
>>>                 mg
>>>
>>>
>>>
>>>                 On 12.01.2018 08:01, Paul King wrote:
>>>>                 @ImmutableCore is similar to @ImmutableBase -
>>>>                 probably okay but I don't think ideal. Another
>>>>                 alternative would be @ImmutableInfo or have an
>>>>                 explicit marker interface with a different package,
>>>>                 e.g. groovy.transform.marker.Immutable but that
>>>>                 might cause IDE completion headaches. Perhaps
>>>>                 @KnownImmutable as a straight marker interface
>>>>                 might be the way to go - then it could be used
>>>>                 explicitly on manually created immutable classes
>>>>                 and avoid the need to use the
>>>>                 knownImmutableClasses/knownImmutables annotation
>>>>                 attributes for that case.
>>>>
>>>>                 Cheers, Paul.
>>>>
>>>>                 On Thu, Jan 11, 2018 at 9:34 PM, mg
>>>>                 <mgbiz@arscreat.com <mailto:mgbiz@arscreat.com>>
wrote:
>>>>
>>>>                     Hi Paul,
>>>>
>>>>                     great to make @Immutable more fine granular /
>>>>                     flexible :-)
>>>>
>>>>                     what about
>>>>                     @ImmutabilityChecked
>>>>                     or
>>>>                     @ImmutableCore
>>>>                     instead of @ImmutableClass ?
>>>>
>>>>                     Cheers
>>>>                     mg
>>>>
>>>>                     -------- Ursprüngliche Nachricht --------
>>>>                     Von: Paul King <paulk@asert.com.au
>>>>                     <mailto:paulk@asert.com.au>>
>>>>                     Datum: 11.01.18 08:07 (GMT+01:00)
>>>>                     An: dev@groovy.apache.org
>>>>                     <mailto:dev@groovy.apache.org>
>>>>                     Betreff: Making @Immutable a meta-annotation
>>>>
>>>>
>>>>                     There has been discussion on and off about
>>>>                     making @Immutable a meta-annotation (annotation
>>>>                     collector) in much the same way as @Canonical
>>>>                     was re-vamped. (This is for 2.5+).
>>>>
>>>>                     I have a preliminary PR which does this:
>>>>                     https://github.com/apache/groovy/pull/653
>>>>                     <https://github.com/apache/groovy/pull/653>
>>>>
>>>>                     Preliminary because it still needs a bit of
>>>>                     refactoring to reduce some duplication of code
>>>>                     that exists between the normal and immutable
>>>>                     map and tuple constructors. I still need to do
>>>>                     this but that can happen transparently behind
>>>>                     the scenes as an implementation detail if we
>>>>                     don't finish it straight away. As well as
>>>>                     reducing duplication, the pending refactoring
>>>>                     will enable things like the pre and post
>>>>                     options for MapConstructor and TupleConstructor
>>>>                     which aren't currently working.
>>>>
>>>>                     I am keen on any feedback at this point. In
>>>>                     particular, while most of the functionality is
>>>>                     pushed off into the collected
>>>>                     annotations/transforms, I ended up with some
>>>>                     left over checks which I kept in an annotation
>>>>                     currently called @ImmutableClass. I tried
>>>>                     various names for this class, e.g.
>>>>                     @ImmutableBase and @ImmutableCheck but finally
>>>>                     settled on @ImmutableClass since the annotation
>>>>                     causes the preliminary checks to be performed
>>>>                     but also acts as a marker interface for the
>>>>                     MapConstructor and TupleConstructor transforms
>>>>                     to do the alternate code needed for
>>>>                     immutability and to indicate that a class is
>>>>                     immutable when it might itself be a property of
>>>>                     another immutable class. Let me know if you can
>>>>                     think of a better name or have any other feedback.
>>>>
>>>>                     Cheers, Paul.
>>>>
>>>>
>>>
>>>
>>>
>>
>>
>
>


Mime
View raw message