Explanations below.
On Tue, Jan 16, 2018 at 12:56 AM, MG <[email protected]
<mailto:[email protected]>> 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 <[email protected]
<mailto:[email protected]>> 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 <[email protected]>
<mailto:[email protected]>
Datum: 13.01.18 13:17 (GMT+01:00)
An: MG <[email protected]> <mailto:[email protected]>
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 <[email protected]
<mailto:[email protected]>> 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
<[email protected] <mailto:[email protected]>> 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 <[email protected]
<mailto:[email protected]>>
Datum: 11.01.18 08:07 (GMT+01:00)
An: [email protected]
<mailto:[email protected]>
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.
