Support for multiple constructors
Tuukka Mustonen
What might be the status for supporting multiple constructors? I am
using JPA, which requires a non-variable constructor but I also want
to have @NonNull generated constructor. Manually adding the non-args
constructor wipes off the Lombok generated constructor, which forces
me to manually type that as well :/
There was an earlier conversation:
...but I couldn't find the reply to all button, so I'm starting a new
thread.
Regards,
Tuukka
PS. Thanks for bringin in this project! Would you see any benefits in
using this in Groovy/Scala environments (both offer a brief syntax to
defining JavaBeans) or is this for plain Java only?
Reinier Zwitserloot
someone wants to extend the concept, we're all ears, but I doubt we're
going to be doing most of the work on that.
We've visited and revisited and rerererererevisited constructor
generation for @Data and we're still not entirely sure how to do it.
We're working on the Flying Turtle release right now which is a lot of
background work on making the base more robust, so we're not currently
looking too much into updating the transformers themselves. We'll get
back to that and tackle this issue when we've delivered this update :)
On Mar 17, 1:06 pm, Tuukka Mustonen <tuukka.musto...@gmail.com> wrote:
> Hi everyone,
>
> What might be the status for supporting multiple constructors? I am
> using JPA, which requires a non-variable constructor but I also want
> to have @NonNull generated constructor. Manually adding the non-args
> constructor wipes off the Lombok generated constructor, which forces
> me to manually type that as well :/
>
> There was an earlier conversation:
>
> http://groups.google.com/group/project-lombok/browse_thread/thread/e6...
Tuukka Mustonen
Regarding support for multiple constructors, why wouldn't you either:
1) Support creation of no-args constructor in addition to @NonNull based creation. As JPA requires this (maybe Hibernate, JDO and others as well?) I suppose this is a very common case. This could be achieved easily(?) by
@Data(noArgsConstructor = true) or @Data(constructor = SomeEnum.NO_ARGS)
or
@Constructor(args = false)
or something like that? I think the point here is that creating a no-args constructor doesn't require anything special. Just throw it in there :)
2) Allow people to manually define additional constructors without them wiping of the Lombok generated one(s). Is there a technical reason for not allowing this currently?
Regards,
Tuukka
--
You received this message because you are subscribed to the Google
Groups group for http://projectlombok.org/
To post to this group, send email to project...@googlegroups.com
To unsubscribe from this group, send email to
project-lombo...@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/project-lombok?hl=en
Reinier Zwitserloot
Moandji Ezana
However, the current constructor generation isn't really something we're proud of either. It only includes variables that are either final or non-null, skipping variables that are fully mutable. That seems arbitrary and doesn't feel right.
Tuukka Mustonen
It is true, that JPA specific issues would better be separated into an extension/plugin. However, having a noargs constructor _may_ be a very common need (just a guess) and implementation should be easy, so supporting that would in vanilla product would sound logical to me. So, I don't suggest fully-featured constructor definitions (because they are problematic and require very verbose syntax) but I suggest a support for creation of simple noargs constructors. Other JPA/JDA/iBATIS/Hibernate/whatever support should be separated into an extension (reading @Column(nullable = true/false) and such attributes).
Or then, maybe the better approach would be to really move those features into extensions but allow this as suggested in my previous mail:
2) Allow people to manually define additional constructors without them wiping of the Lombok generated one(s). Is there a technical reason for not allowing this currently?
On Thu, Mar 18, 2010 at 9:01 AM, Reinier Zwitserloot <rein...@gmail.com> wrote:
However, the current constructor generation isn't really something we're proud of either. It only includes variables that are either final or non-null, skipping variables that are fully mutable. That seems arbitrary and doesn't feel right.It feels exactly right, to me. Final fields are required and must be given to the constructor, everything else is optional, though it might have a default value.
I must totally agree on this. Of course having additional constructors would be nice in order to write:
MyObject o = new MyObject(1, 2, 3);
and not
MyObject obj = new MyObject(1);
obj.setSecond(2);
obj.setThird(3);
What about creating setters in such a way that:
public MyObject setSecond(Integer second) {
this.second = second;
return this;
}
This would allow to do:
MyObject obj = new MyObject(1)
.setSecond(2)
.setThird(3);
Though not much less verbose, a bit at least. But I suppose that's not legal setter for a JavaBean...
Tuukka
The no-args constructor for JPA frameworks is a need, but I think it should be addressed in a JPA-aware plugin, as it is a need that arises from tools, rather than from the language itself. It would, for example, be sufficient to pick up on the @Entity annotation and add a protected no-arg constructor, if none exists.All the proposals so far just seem ugly, which should be reason enough to keep it out. A JPA plugin could also serve as testing ground for this kind of feature, before deciding if it's worth putting into the core and if so, how.Moandji
--
Moandji Ezana
2) Allow people to manually define additional constructors without them wiping of the Lombok generated one(s). Is there a technical reason for not allowing this currently?What do you think about this?
What about creating setters in such a way that:
public MyObject setSecond(Integer second) {
this.second = second;
return this;
}
mkimberlin
is appropriate in most circumstances. However, there are certainly
situations where another constructors are needed. I don't think that
the idea of leaving no-arg constructors to particular plugins is the
appropriate solution though. That requirement is cross-cutting enough
across various frameworks that it would be being implemented
repeatedly.
What about an annotation like this:
@Constructor - for no args
@Constructor({"this", "that", "theother"}) - for a constructor that
includes the fields this, that and theother
If you want/need another constructor, you add another annotation. I
can't think of a good reason that you would ever need more than two of
these (if that), so it shouldn't cause too much 'annotation pixie-
dust' sprinkled on the class.
-michael
Moandji Ezana
@Constructor({"this", "that", "theother"})
Reinier Zwitserloot
way this problem can even occur is if you use @NonNull annotations,
and with the (public) no-args constructor, those @NonNull annotations
aren't actually guaranteed. Isn't it more technically correct to just
remove the annotations? I sort of like the idea of triggering off of
@Entity to also generate a no-args constructor if possible, mostly
because the entire JPA POJO concept is exactly the kind of thing that
causes screwed up class design, such as @NonNull annotations on fields
that can nevertheless end up as null due to a required no-args
constructor.
setters returning 'this' has come up many times before. We're not
going to do it for two reasons:
1) A setter is a well-defined concept in the java ecosystem that does
*NOT* include returning itself. If lombok did generate such setters
that would be very surprising. As you say, it's not legal according to
the javabean spec.
2) More usually, self-returning methods omit the 'set' in their name.
We might be up for something like @Data(fluent=true) with the same
parameter allowed on @Getter and @Setter. This would mean that:
The 'get'/'is' and 'set' aren't included. You get "fieldName()" to get
and "fieldName(Type newValue)" to set, and the setter returns self.
We could also keep lombok's constructor when you write your own but
this has two problems:
1) How do you tell lombok NOT to generate it? If there's no way you'd
have to take out your @Data annotation, add back @EqualsAndHashCode
and @ToString, and @Getter and @Setter on each field, which is rather
a lot of work.
2) Lombok (currently) runs without resolution. It's a really bad idea
to overload a method (or a constructor) with the same # of arguments
where one of the arguments is a subclass of the other and the rest are
equal. However, lombok cannot detect that this is happening, so the
rule would have to be that lombok ONLY skips generating its own
constructor when you've got your own constructor with a parameter list
that matches every type down to the last character. Which would be
inconsistent with the rules for methods (where I strongly feel lombok
should not generate a method if you have one with the same name. We
might revisit that when we have resolution so we can check if there's
no chance of confusion between the two overloaded methods).
The current constructor situation is not optimal, but we'd like to
replace it with something that feels right. None of these solutions so
far feel completely correct. If at some point we must conclude there
is no right answer, then we'll go with these suggestions, but we still
have hope there's an answer out there.
On Mar 18, 11:04 am, Tuukka Mustonen <tuukka.musto...@gmail.com>
wrote:
> I can understand the pains of syntax/approach for creating multiple
> constructors. If there's no neat solution, maybe it shouldn't be implemented
> at all.
>
> It is true, that JPA specific issues would better be separated into an
> extension/plugin. However, having a noargs constructor _may_ be a very
> common need (just a guess) and implementation should be easy, so supporting
> that would in vanilla product would sound logical to me. So, I don't suggest
> fully-featured constructor definitions (because they are problematic and
> require very verbose syntax) but I suggest a support for creation of simple
> noargs constructors. Other JPA/JDA/iBATIS/Hibernate/whatever support should
> be separated into an extension (reading @Column(nullable = true/false) and
> such attributes).
>
> Or then, maybe the better approach would be to really move those features
> into extensions but allow this as suggested in my previous mail:
>
> *2) Allow people to manually define additional constructors without them
> wiping of the Lombok generated one(s). Is there a technical reason for not
> allowing this currently?*
>
> What do you think about this?
>
> On Thu, Mar 18, 2010 at 10:19 AM, Moandji Ezana <mwa...@gmail.com> wrote:
> > Groups group forhttp://projectlombok.org/
>
> > To post to this group, send email to project...@googlegroups.com
> > To unsubscribe from this group, send email to
> > project-lombo...@googlegroups.com<project-lombok%2Bunsubscribe@go oglegroups.com>
Reinier Zwitserloot
1. You may not annotate one node with the same annotation more than
once. Therefore, either this @Constructor annotation will only let you
create 1 constructor, which sort of defeats the point, or, we do
something a bit nuts. For example:
@Constructor("this, that, theother") //Generate one constructor with
listed fields
@Constructor({"this, that, the other", "") //Generate that one AND a
noargs one.
or:
@Constructors({
@Constructor("this", "that", "theother"),
@Constructor
})
and possibly allowing just 1 @Constructor as well if one is all you
need, but this makes it non-obvious what someone should do when they
want another constructor. The jump to "wrap @Constructor in
@Constructors and THEN add the second one" is not exactly something
that's going to casually pop into your mind.
Last but certainly not least, including field names is a code smell.
We're already unhappy with our choice to go with enumerating field
names in strings for the exclude/of parameters of EqualsAndHashcode
and ToString.
mkimberlin
thing. Not sure where my head was...
I agree that field names as strings are generally a code smell.
However, it's also the most common way to reference fields in
annotations that are not field level or to reference fields from other
objects in annotations. You simply can't reference them directly and
any other approach would require making some sort of constant mapping
similar to the @InConstructor approach Reinier mentioned which would
be confusing, I think. It's definitely more resistant to common
refactorings, but it gives the impression of naming constructors
arbitrarily. I'm inclined to think that a numbering would be less
confusing and error prone than names if that approach were used.
Moandji: I should note that I'm not the one who asked for this
feature. I was simply taking part in the conversation. I don't think
writing both is "so bad", personally. I have no problem with leaving
this feature out altogether and having things remain as they are. I
was simply introducing a thought to the conversation.
That said, the more I consider this the more I think that a single
customizable constructor plus a no-arg constructor should be plenty
for almost all situations, no?
So if this feature was deemed necessary, you could always use a
combination of @InConstructor and @Constructor but limit it to a
single @Constructor annotation.
So for a custom constructor:
public class Bogus {
@InConstructor private string thisOne;
@InConstructor private string thatOne;
@InConstructor private string theOtherOne;
}
or if you need a no-arg as well:
@Constructor(noarg=true)
public class Bogus {
@InConstructor private string thisOne;
@InConstructor private string thatOne;
@InConstructor private string theOtherOne;
}
or just a noarg:
@Constructor(noarg=true)
public class Bogus {
private string thisOne;
private string thatOne;
private string theOtherOne;
}
or non-null, custom and noarg constructors:
@Constructor(nonnull=true, noarg=true)
public class Bogus {
@InConstructor private string thisOne;
@InConstructor private string thatOne;
@InConstructor private string theOtherOne;
@NonNull private string imNotNull;
}
Something to that effect...
Reinier Zwitserloot
--
You received this message because you are subscribed to the Google
Groups group for http://projectlombok.org/
To post to this group, send email to project...@googlegroups.com
To unsubscribe from this group, send email to
project-lombo...@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/project-lombok?hl=en
To unsubscribe from this group, send email to project-lombok+unsubscribegooglegroups.com or reply to this email with the words "REMOVE ME" as the subject.
mkimberlin
+1
I like this. I think the whole mix How could we ever know for sure if
Sylvia Browne were really psychic. A couple of college students in NY
just checked! tinyurl.com/yz7fq8o match of fields idea would just be
more clutter than it's worth for very little benefit. All, required,
none...anything else is probably just a sign that something you are
doing is stinky.
Good call.
Also, agreed that it doesn't belong in @Data. @Constructors is
probably the most natural and obvious.
Should a feature request issue be created for this? If you think it's
worth implementing, I'll write one up.
> > Groups group forhttp://projectlombok.org/
>
> > To post to this group, send email to project...@googlegroups.com
> > To unsubscribe from this group, send email to
> > project-lombo...@googlegroups.com<project-lombok%2Bunsubscribe@go oglegroups.com>
Reinier Zwitserloot
--Reinier Zwitserloot
Groups group for http://projectlombok.org/
To post to this group, send email to project...@googlegroups.com
To unsubscribe from this group, send email to
project-lombo...@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/project-lombok?hl=en
To unsubscribe from this group, send email to project-lombok+unsubscribegooglegroups.com or reply to this email with the words "REMOVE ME" as the subject.
Moandji Ezana
Reinier Zwitserloot
booleans (which would then be replaced by AccessLevels)? We can
actually tell the difference between explicitly stated parameters and
defaults. Therefore by default 'minimal' can be on PUBLIC and the
other two on 'none', but if you specify some other constructor then
minimal defaults to NONE too unless you are explicit about it. Then
you get:
@Constructors - 1 public minimal constructor (implicit if you use
@Data)
@Constructors(noargs=AccessLevel.PROTECTED) - 1 protected noargs and
nothing else.
@Constructors(noargs=AccessLevel.PROTECTED,
minimal=AccessLevel.PUBLIC) - 1 protected noargs, 1 public standard
(minimal).
Feels a bit wordy.
How about:
@Constructors({ConstructorTypes.PROTECTED_NOARGS,
ConstructorTypes.MINIMAL})
Oi vey, that's as long as the AccessLevel based one and actually feels
uglier (and is less flexible unless we add every combination of
constructor type and access level to the enum).
So, this:
@Constructors(noargs=AccessLevel.PROTECTED,
minimal=AccessLevel.PUBLIC)
is the winner then? I'd love some more feedback before we go for it.
mkimberlin
option...then it's only:
@Constructors(noargs=PROTECTED, minimal=PUBLIC)
I don't think this is too bad. Since this is essentially the same as
the booleans way with the little extra feature of access level control
for (almost) free. I think this is reasonable.
mkimberlin
tweet in that earlier email. oops...not sure how that happened. :)
mkimberlin
type. So one of you will have to change it to Enhancement, I
suppose. I tried to capture the gist of what was said here in the
issue...but, I also provided a link back to this thread, just in case
it's a while before you get around to it and it grows cold in your
mind.
I would offer to give a hand in implementation of it, but I'm afraid
that I have a few too many other pet projects in the air at the
moment. :) Perhaps I'll double back and throw some code toward Lombok
in the future. It's a great project.
Also, Reinier...I would like to pick your brain slightly about the
upcoming release. If there are any significant changes to the
features/functionality of Project Lombok, I would like to update the
article that I wrote on it to include them. Life hasn't given me much
opportunity to keep a close eye on the project, so I'm not sure what
I've missed in the last few months.
-michael
On Mar 19, 8:01 am, Reinier Zwitserloot <reini...@gmail.com> wrote:
Tuukka Mustonen
I don't think having @Constructor for single constructor and switching to @Constructors({@Consutrctor, @Constructor}) would be a bad thing. It is somewhat verbose but it's the common solution to having multiple similar annotations so shouldn't it be well known? If not, users can be made aware of it with one or two lines of documentation (and I think it still feels quite logical). Also, as stated previously, referring to fields via strings and reflection is ugly, but yet a very common approach (which, of course, is not to say that it's a good one). However, losing refactoring support is what I, too, shun. So, overall, the current suggestion of supporting max. 3 constructors with different access levels is flexible enough for the majority of use cases...
However, I would still emphasize the importance of allowing custom constructors in addition to the ones created by Lombok. I'm clearly no match to you in technical domain, so I didn't quite get grasp of what Reinier meant by Lombok being "run without resolution" and overriding the constructors with nearly exact match of arguments. So I'm just stating this: the more features Lombok provides, the sadder it is to give those up. While it might not be common case, cannot there be situations where one does need a fourth constructor? The outcome of the currently suggested implementation would be having to manually write all the 4, not just the last fourth. Similar case is when one wants to customize one of the automatically provided constrcutors, for example one that's provided with MINIMAL enum - customization means manually rewriting that constructor (which is ok) but then one would have to manually type in the NOARGS (not too bad) and ALL (very frustrating) constructors as well. What was wrong with the @IgnoredByLombok annotation suggested by Moandji?
Please ignore the above paragraph if the answers and technical reasoning was already provided by Reinier in an ealier post.
Tuukka
Reinier Zwitserloot
say, "String" somewhere in your java code is in fact referring to
"java.lang.String". We do our own half-baked resolution on types so
that we can figure out that @Getter did in fact refer to
@lombok.Getter and not some other sort of getter but it's not 100%
perfect (especially when star imports are involved, lombok can come to
different conclusions that the official java spec). Also, and perhaps
more importantly, we don't have access to the stucture of *other*
source files. Thus, even we do know something is referring to
"java.lang.String", we don't know what methods are in
"java.lang.String". And, more crucially for this particular case, we
don't know that for example "java.lang.Integer" is a subclass of
"java.lang.Number". Therefore, if we wanted to generate:
public MyClass(Integer x) {}
but you already manually typed:
public MyClass(Number x) {}
generating the integer-based one is probably a bad idea, as having
both constructors around is going to be very confusing. However, we
simply don't know that Integer is a subclass of Number (well, for
java.lang. stuff its obvious, but what if it were your own classes and
not those in java.*?)
What we could do is say that we will forego generating our own stuff
only if you have a method with the same name *AND* the same # of
parameters. That would probably solve most cases, but it wouldn't be
all that natural (you wouldn't know unless you read about it). Then
again, neither is the current situation (where lombok does not
generate something if you have a method with the same name regardless
of parameters).
On Mar 21, 2:17 pm, Tuukka Mustonen <tuukka.musto...@gmail.com> wrote:
> Thank you guys for taking initiative and so eager attitude towards this :)
>
> I don't think having @Constructor for single constructor and switching to
> @Constructors({@Consutrctor, @Constructor}) would be a bad thing. It is
> somewhat verbose but it's the common solution to having multiple similar
> annotations so shouldn't it be well known? If not, users can be made aware
> of it with one or two lines of documentation (and I think it still feels
> quite logical). Also, as stated previously, referring to fields via strings
> and reflection is ugly, but yet a very common approach (which, of course, is
> not to say that it's a good one). However, losing refactoring support is
> what I, too, shun. So, overall, the current suggestion of supporting max. 3
> constructors with different access levels is flexible enough for the
> majority of use cases...
>
> However, I would still emphasize the importance of allowing custom
> constructors *in addition* to the ones created by Lombok. I'm clearly no
> > Groups group forhttp://projectlombok.org/
>
> > To post to this group, send email to project...@googlegroups.com
> > To unsubscribe from this group, send email to
> > project-lombo...@googlegroups.com<project-lombok%2Bunsubscribe@go oglegroups.com>
Tuukka Mustonen
It sounds like Lombok cannot, in worst case, do anything critical? The worst case (actual error) would be to create duplicate constructor (with identical # of arguments and identical "appearances"). This is something that Lombok can find out and avoid, right?
About the quite-similar-but-not-similar signatures. What's wrong with:
public MyClass(Integer x) {}
and
public MyClass(Number x) {}
Both constructors are sane, though having 2 so similar signatures might be confusing, but user should be aware of both of them, anyway. Having such a case would be quite rare, also? I fail to see any actual problems in this?
Why not just generate the constructors as indicated via annotations (previous discussion) and retain generation on those that have exactly same signature already manually defined?
Humble regards,
Tuukka
Groups group for http://projectlombok.org/
To post to this group, send email to project...@googlegroups.com
To unsubscribe from this group, send email to
project-lombo...@googlegroups.com
To unsubscribe from this group, send email to project-lombok+unsubscribegooglegroups.com or reply to this email with the words "REMOVE ME" as the subject.
Reinier Zwitserloot
--Reinier Zwitserloot
Tuukka Mustonen
So how would you turn _off_ the lombok-generated constructor?
Also, to be consistent, if we take this stance for constructors (generate the constructor unless an explicit constructor with the *Exact* same parameter list exists), then we should also take this stance for all methods generated by lombok (getters, setters, equals, and hashCode, and perhaps more importantly, all future methods we might want to add at some point). That's quite an impact. Because the methods/constructors generated by lombok are far less visible than explicit ones, and overloading with the same # of parameters where each parameter type is equal or one is a subtype of the other is a really bad anti-pattern, so I certainly don't feel entirely comfortable letting lombok actively commit such atrocities and trusting the programmer to 'figure it out'.I'd see more bread personally in some sort of general lombok annotation that you can stick on any method/constructor that says: It's allright, just generate your stuff, I'll vouch for this method/constructor not conflicting with whatever lombok is planning.
Reinier Zwitserloot
automatically, but what if you wanted lombok to generate everything
_except_ that constructor? Right now, you define your own constructor
and lombok will automatically choose to not generate it. The same goes
for methods.
On Mar 26, 7:26 am, Tuukka Mustonen <tuukka.musto...@gmail.com> wrote:
> ...
>
> read more »
Maaartin
> No, I mean, lombok's @Data should obviously generate a constructor
> automatically, but what if you wanted lombok to generate everything
> _except_ that constructor? Right now, you define your own constructor
> and lombok will automatically choose to not generate it. The same goes
> for methods.
Probably I'm missing something (although I've just read the whole
thread), but I think this behavior is just fine and should not and
need not change. If there's a @Constructors annotation, then lombok
should generate exactly the constructors specified. In this case there
should be no check for existing constructors, no resolution, etc. So
if you specify
Constructors({noargs=AccessLevel.PROTECTED})
then you always get a protected noargs constructor generated and no
others (regardless of what constructors already exists). As a special
case, using
Constructors({})
could be used to switch the generation off (quite useless since
defining any ctor does it as well).
Maaartin-1
default it should do a shallow copy of all fields, this is what I need
most of the time. This is usually quite simple, it could handle
primitive types, arrays, and all build-in collections easily. Obviously,
immutable types do not need to be copied.
There should be a possibility to handle other types, e.g., a field
MyFancyCollection myCol;
could be handled by the method cloneMyFancyCollection(MyFancyCollection)
defined in the processed class. The method could be inherited or private
or whatsoever. Alternatively, lombok could look for a method with a name
based on the field name, or on an annotation on the field.
I can imagine to allow sharing of some field, or to require deeper copy
of it, this could be handled by a cloneXXX method, too.
The clone constructor is often quite useful and quite boring to write,
moreover it's easy to forget a field. It's an alternative to the clone()
method and sometimes the only way how to implement it correctly.
What do you think about it?
Reinier Zwitserloot
we'd have to take some liberties here, and say that clones are
shallow. Folks should be able to figure out from there that this is
irrelevant for immutables. We could take it upon ourselves to roll our
own clones for arrays and stuff from java.util.collections but that
seems a bit arbitrary. We could attempt to deep-clone everything,
requiring either some sort of flag (You may shallow-clone this field)
or a method with a special name like you suggest, for all types that
we don't know how to clone ourselves, but none of this is very
consistent; for example, ArrayList's clone constructor does a
_SHALLOW_ copy; it'll create a new list but the new list is populated
with references to the exact same objects that were in the source
list, even if those objects are Clonable.
The most consistent approach is to say that the whole thing will be
shallow-copied, though this would have to extend to lists. We could
then offer opt-out behaviour: Either annotate a field, or offer a
magically named method, to write your own clone algorithm for a
certain member. Then if you wanted your lists to be semi-shallow (list
is cloned, members aren't), you could write just the "new
ArrayList<T>(oldList)" part of it yourself. Exactly what would this
look like though? Also, with annotations, as they can't contain code,
you could really only annotate Clonables.
Maaartin-1
> Not a bad idea. Unfortunately we can't detect immutable objects, so
> we'd have to take some liberties here, and say that clones are
> shallow.
I mentioned the immutables only as an optimization, as it makes no sense
to clone String, Integer, or BigInteger. But I'd prefer to clone them
to not cloning e.g., arrays (s. below).
> Folks should be able to figure out from there that this is
> irrelevant for immutables. We could take it upon ourselves to roll our
> own clones for arrays and stuff from java.util.collections but that
> seems a bit arbitrary.
Agreed, but a cloned object with shared lists etc. is almost never
usable. Since the whole ctor gets generated, there's no way to fix it
manually afterwards (assuming the fields being final).
> We could attempt to deep-clone everything,
> requiring either some sort of flag (You may shallow-clone this field)
> or a method with a special name like you suggest, for all types that
> we don't know how to clone ourselves, but none of this is very
> consistent; for example, ArrayList's clone constructor does a
> _SHALLOW_ copy; it'll create a new list but the new list is populated
> with references to the exact same objects that were in the source
> list, even if those objects are Clonable.
Yes, but that's a quite different issue. IIRC, I've never needed a
shallow copy and I've never needed an infinitely deep one. Whenever I
cloned an Object I need it to be quite independent of the original in
the sense that calling methods on the copy doesn't influence the
original, but methods should (according to Demeter) only modify the
object properties themselves, not the things contained in them. That's
why depth-1 cloning is nearly always what I needed. OTOH, for
general-purpose containers is shallow copy probably the best. Specifying
the depth in the @Constructor annotation is probably the way to go.
> The most consistent approach is to say that the whole thing will be
> shallow-copied, though this would have to extend to lists.
IMHO, the depth-1 copy is as consistent as the shallow or infinitely
deep one, just more useful for user-defined objects.
I've just had a look at java.util and java.util.concurrent and it looks
like all the collections and maps have a clone constructor.
Unfortunately, the concurrent ones are not Cloneable, and the others'
method clone() returns Object. The clone constructors performs a shallow
copy, which is what you need for containers most of the time.
> We could
> then offer opt-out behaviour: Either annotate a field, or offer a
> magically named method, to write your own clone algorithm for a
> certain member.
I'd prefer specifying the default depth in the class annotation, with
the possibility to override it using member annotation.
Then if you wanted your lists to be semi-shallow (list
> is cloned, members aren't), you could write just the "new
> ArrayList<T>(oldList)" part of it yourself. Exactly what would this
> look like though? Also, with annotations, as they can't contain code,
> you could really only annotate Clonables.
You could select one the following possibilities for object o of class C:
- o
- o.clone()
- new C(o)
- MyCloner.cloneOf(o), with MyCloner.class given as an annotation parameter
Actually, I could be happy with always calling MyCloner, where I could
configure it myself. This could be easy for you to do (at least when
using a string instead of a class as a parameter) and it would be the
most flexible solution. Ideally something like
@CloneConstructor(access=AccessLevel.PRIVATE,
cloneMethod=AccessLevel.PUBLIC, cloner=MyCloner.class)
which would lead to something like
class MyClass {
private Date d;
private String s;
private final List<String> list;
private MyClass(MyClass other) {
d = (Date) MyCloner.cloneOf(other.d);
s = (String) MyCloner.cloneOf(other.s);
list = List<String> MyCloner.cloneOf(other.list);
}
public MyClass clone() {
return new MyClass(this);
}
}
Implementing MyCloner is a trivial exercise for the user. I don't know
how the lombok code gets generated, if it was something like source code
then I'd define special methods for each type, otherwise I'd switch on
the types. Maybe something like follows would be better:
d = MyCloner.cloneOf(Data.class, other.d);
Reinier Zwitserloot
fill it with shallow clones of each field? Does that mesh with what
folks might expect? Could be that's true, in general I program with so
many immutables I don't really notice much. Specifying depth class-
wide seems strange; if I have:
A: Object/MutableType1/Field1
B: Object/MutableType1/Field2
C: Object/MutableType2/Field1
D: Object/MutableType2/Field2
then if I am allowed to mess with how far cloning should go in the
first place, then why not let me say that A should be fully cloned, B
only 2-deep, C and D only 1-deep?
If we call cloner code you write yourself, then I'm not sure lombok
even adds any meaningful boilerplate reduction anymore; might as well
just write your own cloning constructor. I thoroughly dislike magic
signatures; they should only be used as a last resort.
"cloneOf(Type.class, instance)" is definitely a magic signature.
There's just no way for you to know that's how you're supposed to make
them without delving deep into the documentation, and that's really
not a good thing. It can be excusable if its the only sane way to
implement a killer feature, but this isn't a killer feature. I'd have
to ask Roel but for me, this feature is sufficiently borderline that
whatever magic we end up doing should be completely natural and
require next to no configuration. If that's not in the cards then
we're out of luck.
Maaartin-1
> So the plan is that by default, a clone will create a new object, and
> fill it with shallow clones of each field? Does that mesh with what
> folks might expect?
I'd say no, but feel free to introduce a required parameter, so nobody
would be surprised. Or do a shallow copy by default and let them use
their own cloner. This is what I prefer now (see the example below).
Shallow copy is what Object.clone() does so there is no surprise.
> Could be that's true, in general I program with so
> many immutables I don't really notice much.
So do I whenever possible.
> Specifying depth class-
> wide seems strange; if I have:
>
> A: Object/MutableType1/Field1
> B: Object/MutableType1/Field2
> C: Object/MutableType2/Field1
> D: Object/MutableType2/Field2
>
> then if I am allowed to mess with how far cloning should go in the
> first place, then why not let me say that A should be fully cloned, B
> only 2-deep, C and D only 1-deep?
Sure, but this can't be done by a simple class-wide annotation. So you
could allow a global specification in a class-wide template which could
be overridden by field annotations if necessary. I'm going to modify my
example to account for this.
> If we call cloner code you write yourself, then I'm not sure lombok
> even adds any meaningful boilerplate reduction anymore; might as well
> just write your own cloning constructor.
I strongly disagree.
1. In case you need only a shallow copy then your cloner is a class with
single function implementing identity.
2. You could provide some useful cloners solving most common problems. I
fact I've wrote something appropriate for most cases.
3. No matter how complicated you cloner is, you can reuse it more for
many fields in many classes.
Recently, I saw a library containing over 100 classes (implementing
cryptographic hash functions), most of them containing a clone method.
All used fields are either primitives or arrays of them, so only arrays
need to be handled. Writing such a cloner took me less than 5 minutes
and it could save 1000+ lines of code in the library mentioned.
> I thoroughly dislike magic
> signatures; they should only be used as a last resort.
> "cloneOf(Type.class, instance)" is definitely a magic signature.
> There's just no way for you to know that's how you're supposed to make
> them without delving deep into the documentation, and that's really
> not a good thing.
How is it any more magical then any other method? Probably you assume
the method to be static, but it needn't be (see my example below).
The class parameter is not necessary, I left it out for now.
> It can be excusable if its the only sane way to
> implement a killer feature, but this isn't a killer feature. I'd have
> to ask Roel but for me, this feature is sufficiently borderline that
> whatever magic we end up doing should be completely natural and
> require next to no configuration. If that's not in the cards then
> we're out of luck.
I hope you change you mind after having look at my example. The cloning
is a lot of boilerplate and quite error-prone. With the interface I
introduced now the magic is gone IIUYC, the only remaining thing is the
noarg-ctor requirement, but this is quite common.
I created quite a large example containing my new proposal for ctor
annotations which includes static factory methods as well. Because of
this, I'd prefer separate annotations for each constructor. See
http://dl.dropbox.com/u/4971686/lombok/cloner-100630-110303.zip
Maaartin-1
than expected, because of the infinite chaos in Java API. Some of the
classes provides a sort of copy constructor, others have a clone()
method returning for whatever reason Object. However, writing a cloner
for the most important cases is easy and doing it for all mutable
classes in say java.lang, java.util, and java.util.concurrent is doable.
Such a cloner could be useful outside of lombok, too.
Customizing the behavior for fields could be easily done using
@Retention(RetentionPolicy.SOURCE)
@Target(ElementType.FIELD)
public @interface FieldCloner {
Class<? extends Cloner> value();
}
for overriding the default cloner.
Reinier Zwitserloot
listed in an interface, you have no idea what you're supposed to
implement without looking at javadoc. the main method is an example of
a magic method: Unless someone tells you that's what you should write,
there's no hint. If java were redesigned today I'm fairly sure we'd
have an interface (java.lang.Application or some such), a requirement
to have a no-args constructor, and you can run any subtype of
Application. Serialization is also rife with magic signatures; the
serialVersionUID field as well as the magic signatures for readObject
and writeObject. These things invariably show up in "how not to write
APIs" examples, for good reason.
If we're going to support this I'm strongly leaning towards the
following concept:
The annotation to generate a cloning constructor itself will by
default create a new object and assign to each field the cloned
version of each current variable. I complained before that this
doesn't appear to be particularly expectable, but now I'm having
second thoughts on that concern; in general this always seems to be
'the right thing'. ArrayList's cloner will for example copy all
internal primitives (length and size), as well as the backing array,
but not the elements in it. That's exactly this: Create a new
ArrayList and assign to each field in it a clone of the original.
We'll of course be smart enough to know that there's no need to clone
BigInteger, BigDecimal, java.lang.Integer and wrapper friends, all
primitives, and Strings. arrays and lists are already cloneable so
that works out nicely. If any of your fields aren't cloneables, the
annotation will generate a compile time error. Possibly there will be
another annotation you can stick on a field to say that this
particular field does not need cloning, and possibly there will be a
flag on the constructor annotation to say you want a completely
shallow clone.
If the above cannot be used to arrive at the desired constructor, then
you'd have to write it from scratch. I get the feeling the above
should cover easily 80%+ of all required copy constructors, and adding
loads of complex interactions including either magic signatures or
extra interfaces (which would also lead to runtime dependencies,
something we try to avoid), does not "feel" like lombok. We tackle
boilerplate that is both (A) annoying and hard to get right, and (B)
common. Once some annoying and hard to get right boilerplate ceases to
be particularly common, it's out of lombok's current aims. We've got
more pressing boilerplate to fry :P
While the right way to clone an arraylist is normally: new
ArrayList<T>(otherList);, you can also clone it with:
(ArrayList<T>)otherList.clone(); - this is nice and general.
We can't do the above plan yet, though - in order to know that
"ArrayList" is in fact referring to "java.util.ArrayList" we'd
preferably have full resolution. Right now we have lightweight
resolution, based on our own interpreting of the import list, but it's
not perfect. Also, it is completely incapable of figuring out if any
given class is Cloneable, so if we build it with today's lombok we
could only insert a runtime instanceof check.
Graham Allan
>
> To: Project Lombok <project...@googlegroups.com>
>
> Not a bad idea. Unfortunately we can't detect immutable objects, so
> we'd have to take some liberties here, and say that clones are
> shallow.
<offtopic>
I'm currently working on a project which will allow this kind of investigation
of classes. This isn't a suggestion to use it for this purpose, I just thought
(hoped) you might be interested in taking a look:
Mutability Detector
http://code.google.com/p/mutability-detector/
It's designed to analyse Java classes and report on whether they're immutable
or mutable. If your curiosity is piqued, and you get the chance to take a
look, I'd love to hear your thoughts on it.
Regards,
Graham
Reinier Zwitserloot
To elaborate:
Let's say we have a File abstraction. java.io.File will do nicely. It
looks, for all intents and purposes, immutable; there is no way to
change the fields that exist inside a java.io.File object once it is
created; you can only create new File objects as offshoots from the
original one, similar to how .toLowerCase() produces new strings.
However, calling a java.io.File object immutable is disingenuous; its
state and its behaviour most assuredly can be changed over time; for
example, call .delete() and from that point onwards, .exists() returns
something else. Partly because of this, treating java.io.File as
universally thread-safe doesn't really work; if one thread creates a
file at around the same time another thread is calling .delete() on
it, you don't know what's going to happen, and that's not a good idea.
In this sense, instead of calling something 'immutable', one could
instead say its more useful to call something 'side effect free',
which is mostly a superset of immutable (because changing something is
a side effect). SEF is also a method property and not an object
property; Strings are immutable, sure, but even if they
weren't, .toLowerCase() is definitely side effect free. ArrayList is
NOT immutable, but ArrayList.size() is side-effect-free. One useful
property of SEF methods is that calling them and ignoring the return
value is necessarily a bug.
Unfortunately some things feel like side effects but aren't. For
example, logging is clearly not SEF, but one should treat it as such.
However, immutables by themselves can easily be non-SEF, and in
general when this occurs the immutable is NOT thread safe. For
example, let's say there's an object that is (shallowly) immutable by
trivial introspection: Every single field it has is final. If one of
those fields is an ArrayList, then all bets are off and treating it as
an immutable is a mistake, even though it may look like one. One could
easily get around this problem by also checking if all final fields
are also themselves immutable, but this doesn't really help; one can
always create a fake mutable variable by using a weak identity hash
map, or just in general interacting with static stuff that isn't
immutable/SEF. What one cannot get around, however, is that writing to
such a map is a side effect.
I'm interested in how you've tackled these issues :P
Personally I've always considered solutions in this space to be most
useful if they focus on SEF and ignore immutability as a driver. Any
method can then be marked SEF or not and a method is SEF if it calls
only SEF methods and does not change any state itself. This then
allows for e.g. native methods that change no state to be marked as
SEF, native methods that DO change state (e.g. File.delete()) to be
marked as non-SEF, and for certain non-SEF methods to be shadowed as
SEF, such as logger calls. However, this does mean SEF methods are not
neccessarily at method level constant (defined as: If you give it the
same parameters and receiver, it'll always return the same value).
"FOO".toLowerCase() is both SEF and method-wise constant, but clearly
List.size() is SEF but certainly not method-wise constant.
SEF, method-wise constant, and immutable all have separate benefits,
but are hard concepts to integrate:
- constant methods can be cached / memoized.
- returning SEFs from inside an API is always totally safe and does
not require cloning/guarding of any sort.
- there's no point in cloning an immutable. It may not be safe,
though. (case in point: You can use a clone of java.io.File to delete
a file just as well; it's not safe to hand em out, but cloning it is
pointless).
And it gets still more complicated: Unless we use final types, one
simply cannot declare an entire type SEF / constant / immutable; some
subclass can always screw up assumptions. It would be fantastic if we
could make types subclassable whilst compiler-enforcing that any
subclasses retain the SEF / constant / immutable property, but java
doesn't have that.
Also, java.lang.Object is already clearly not SEF nor
constant: .wait() and .notify() are exactly the kinds of calls that
only very shallow definitions of immutability find immutable, and
which are certainly neither SEF nor constant. As all objects are
subclasses of java.lang.Object, this would mean defining object-level
SEF/const-ness as: All its methods are SEF / constant, doesn't really
work, as we'd have to exclude wait/notify already.
I've always found this discussion delved into far too much complexity
to be worth it, so I'm interested in how far you went in tackling
these issues. I'd like to be able to support an annotation that says:
Hey, compiler, can you verify for me that this method remains SEF? I
don't want some guy under pressure to deliver a crucial bugfix to
screw up and accidentally give this thing side effects as I'm relying
on it not to have them elsewhere in my code.
On Jul 1, 7:01 pm, Graham Allan <grundlefl...@googlemail.com> wrote:
> > From: Reinier Zwitserloot <reini...@gmail.com>
> > To: Project Lombok <project...@googlegroups.com>
>
> > Not a bad idea. Unfortunately we can't detect immutable objects, so
> > we'd have to take some liberties here, and say that clones are
> > shallow.
> > [snip]
>
> <offtopic>
>
> I'm currently working on a project which will allow this kind of investigation
> of classes. This isn't a suggestion to use it for this purpose, I just thought
> (hoped) you might be interested in taking a look:
>
Maaartin-1
even worse than I thought.
> And it gets still more complicated: Unless we use final types, one
> simply cannot declare an entire type SEF / constant / immutable; some
> subclass can always screw up assumptions. It would be fantastic if we
> could make types subclassable whilst compiler-enforcing that any
> subclasses retain the SEF / constant / immutable property, but java
> doesn't have that.
Theoretically, it could be done using APT or Lombok with an inheritable
@SideEffectFree annotation reporting an error in case of violation. This
assumes you could access the results of a tool doing all the complicated
static analysis, which is not easy.
> Also, java.lang.Object is already clearly not SEF nor
> constant: .wait() and .notify() are exactly the kinds of calls that
> only very shallow definitions of immutability find immutable, and
> which are certainly neither SEF nor constant. As all objects are
> subclasses of java.lang.Object, this would mean defining object-level
> SEF/const-ness as: All its methods are SEF / constant, doesn't really
> work, as we'd have to exclude wait/notify already.
A whole class could be marked as SEF if you could prove that none of
wait(), notify(), notifyAll() and other non-SEF methods get actually
called. This is even more complicated.
Morever, there are things like dynamic proxies, and even a trivial
getter in a class having no compile-time subclasses is not SEF if it
gets proxied by hibernate. Although, assuming an appropriate transaction
isolation level, in a sense it is, as it should return consistent values
when called twice in a row.
And we have AOP and reflection for making things even more complicated.
> I've always found this discussion delved into far too much complexity
> to be worth it, so I'm interested in how far you went in tackling
> these issues. I'd like to be able to support an annotation that says:
> Hey, compiler, can you verify for me that this method remains SEF? I
> don't want some guy under pressure to deliver a crucial bugfix to
> screw up and accidentally give this thing side effects as I'm relying
> on it not to have them elsewhere in my code.
This would be nice, but there should be a possibility for the programmer
to specify not only their assertions but also their
assumptions. For example, taken literally, String is not immutable and
String.hashCode() is not SEF, but for all practical purposes it is. The
programmer should have the possibility to declare his methods/classes to
have a property even when it can't be verified by the tool and the tool
should use these assumptions for further reasoning.
Reinier Zwitserloot
--Reinier Zwitserloot
--
You received this message because you are subscribed to the Google
Groups group for http://projectlombok.org/
To post to this group, send email to project...@googlegroups.com
To unsubscribe from this group, send email to
project-lombo...@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/project-lombok?hl=en
Graham Allan
>
> From: Reinier Zwitserloot <rein...@gmail.com>
> To: Project Lombok <project...@googlegroups.com>
>
Hey Reinier,
Thanks for commenting. Replies inline.
> How did you solve the immutable vs. Side-Effect-Free problem?
>
The simple answer is that I haven't. When I began working on it (as part of a
uni assignment) I used Josh Bloch's rules for immutability which he talks
about in Effective Java 2nd Ed. (e.g. don't provide mutators, make fields
private, etc). With the automatic analysis for those in place, I found that on
real code, they were particularly strict, and few real classes I analysed were
immutable. From then the focus has been on relaxing those rules and being more
sophisticated in recognising mutability.
Though definitely a worthy pursuit, looking to declare code/methods/classes as
SEF hasn't been considered.
> To elaborate:
>
> Let's say we have a File abstraction. java.io.File will do nicely. It
> looks, for all intents and purposes, immutable; there is no way to
> change the fields that exist inside a java.io.File object once it is
> created; you can only create new File objects as offshoots from the
> original one, similar to how .toLowerCase() produces new strings.
>
> However, calling a java.io.File object immutable is disingenuous; its
> state and its behaviour most assuredly can be changed over time; for
> example, call .delete() and from that point onwards, .exists() returns
> something else. Partly because of this, treating java.io.File as
> universally thread-safe doesn't really work; if one thread creates a
> file at around the same time another thread is calling .delete() on
> it, you don't know what's going to happen, and that's not a good idea.
>
> In this sense, instead of calling something 'immutable', one could
> instead say its more useful to call something 'side effect free',
> which is mostly a superset of immutable (because changing something is
> a side effect). SEF is also a method property and not an object
> property; Strings are immutable, sure, but even if they
> weren't, .toLowerCase() is definitely side effect free. ArrayList is
> NOT immutable, but ArrayList.size() is side-effect-free. One useful
> property of SEF methods is that calling them and ignoring the return
> value is necessarily a bug.
>
SEF is definitely a useful property, but I'd like to comment on the j.i.File
example. The reason that the principles of immutability won't work is that the
file system isn't modelled as a normal Java type, instead it dips below the
surface by making native calls. If the file system was a mutable Java type,
which each File instance had a reference to, the mutability would bubble up.
Not that such a tool as I'm trying to create can ignore this kind of thing,
but hopefully most of the time, because mutability isn't hidden in native
calls, potential side-effects will affect the mutable/immutable status of
calling classes.
> Unfortunately some things feel like side effects but aren't. For
> example, logging is clearly not SEF, but one should treat it as such.
>
> However, immutables by themselves can easily be non-SEF, and in
> general when this occurs the immutable is NOT thread safe. For
> example, let's say there's an object that is (shallowly) immutable by
> trivial introspection: Every single field it has is final. If one of
> those fields is an ArrayList, then all bets are off and treating it as
> an immutable is a mistake, even though it may look like one. One could
> easily get around this problem by also checking if all final fields
> are also themselves immutable, but this doesn't really help; one can
> always create a fake mutable variable by using a weak identity hash
> map, or just in general interacting with static stuff that isn't
> immutable/SEF. What one cannot get around, however, is that writing to
> such a map is a side effect.
>
> I'm interested in how you've tackled these issues :P
The mutable field scenario is checked for at the moment. Currently any fields
which are deemed to be mutable render the containing type mutable. This is a
bit hacky, and I'm looking to perform more analysis so that a class can have
mutable fields (such as an instance of ArrayList) but can still be immutable -
provided its never possible to mutate those fields. This is an issue that's
being discussed on the project page at the moment[1]. The issue of detecting
references to static mutable state is also briefly touched upon in that issue's
comments.
I'm not quite sure I entirely understand your point about the weak identity
hash map. However, collections in general are found to be mutable, and
therefore classes which contain them are affected by this. Detecting use of
unmodifiable collections (which appears mutable, but is made immutable at
runtime) is something that has also been considered, but there's been no work
around this.
>
> Personally I've always considered solutions in this space to be most
> useful if they focus on SEF and ignore immutability as a driver. Any
> method can then be marked SEF or not and a method is SEF if it calls
> only SEF methods and does not change any state itself. This then
> allows for e.g. native methods that change no state to be marked as
> SEF, native methods that DO change state (e.g. File.delete()) to be
> marked as non-SEF, and for certain non-SEF methods to be shadowed as
> SEF, such as logger calls. However, this does mean SEF methods are not
> neccessarily at method level constant (defined as: If you give it the
> same parameters and receiver, it'll always return the same value).
> "FOO".toLowerCase() is both SEF and method-wise constant, but clearly
> List.size() is SEF but certainly not method-wise constant.
>
Is this what is the intention of C++'s const keyword?
> SEF, method-wise constant, and immutable all have separate benefits,
>
> but are hard concepts to integrate:
> - constant methods can be cached / memoized.
> - returning SEFs from inside an API is always totally safe and does
>
> not require cloning/guarding of any sort.
>
> - there's no point in cloning an immutable. It may not be safe,
>
> though. (case in point: You can use a clone of java.io.File to delete
> a file just as well; it's not safe to hand em out, but cloning it is
> pointless).
>
>
> And it gets still more complicated: Unless we use final types, one
> simply cannot declare an entire type SEF / constant / immutable; some
> subclass can always screw up assumptions. It would be fantastic if we
> could make types subclassable whilst compiler-enforcing that any
> subclasses retain the SEF / constant / immutable property, but java
> doesn't have that.
There is an aspect of the tool that recognises the complications around
subtyping. For instance, a class X not declared final can still be considered
immutable. However, class Y, a client of the non-final class X, will reflect the
possibility that the X could be subclassed and screw up Y's mutability status.
>
> Also, java.lang.Object is already clearly not SEF nor
> constant: .wait() and .notify() are exactly the kinds of calls that
> only very shallow definitions of immutability find immutable, and
> which are certainly neither SEF nor constant. As all objects are
> subclasses of java.lang.Object, this would mean defining object-level
> SEF/const-ness as: All its methods are SEF / constant, doesn't really
> work, as we'd have to exclude wait/notify already.
>
Rightly or wrongly, I took the path that said j.l.Object is immutable, and the
wait/notify methods are exempt as they: hide below the native surface level;
are low level synchronisation details that don't really represent state as a
developer writing their own classes would see it; and... the thought of
figuring out what it means for every class if j.l.Object is mutable didn't
really bear thinking about :-) NB: this is the same approach taken for
concepts such as reflection and AOP - basically my head's in the sand.
>
> I've always found this discussion delved into far too much complexity
> to be worth it, so I'm interested in how far you went in tackling
> these issues. I'd like to be able to support an annotation that says:
> Hey, compiler, can you verify for me that this method remains SEF? I
> don't want some guy under pressure to deliver a crucial bugfix to
> screw up and accidentally give this thing side effects as I'm relying
> on it not to have them elsewhere in my code.
The goal as I see it at the moment, is to try to create a tool to provide the
most use to developers writing immutable classes. Most of the time (AFAICT)
this generally isn't too difficult, and it mostly screwed up by those errors one
step removed from a compiler error. My thoughts were that instead of providing
annotations and the tools to check at compile time, instead integrate with
unit testing frameworks so that developers can write Assert.isImmutab
le(MyClass.class); This seems the most natural and easily introduced way to
bring in automated mutability analysis. This is still just in the
contemplation state though.
Certainly the points you raise about SEF-ness are relevant and interesting
(and I must say I'm probably falling short of properly understanding the
subtleties atm) but I think they may just be a bit lofty (and the path be
littered with dragons) for what I'm trying to achieve. To summarise it as
succintly as I can, it's not even close to the level of sophistication you're
talking about :-)
Thanks for taking the time to comment, much appreciated.
Regards,
Graham
[1] http://code.google.com/p/mutability-detector/issues/detail?id=3
Reinier Zwitserloot
On Jul 4, 1:15 am, Graham Allan <grundlefl...@googlemail.com> wrote:
>
> I'm not quite sure I entirely understand your point about the weak identity
> hash map. However, collections in general are found to be mutable, and
> therefore classes which contain them are affected by this. Detecting use of
> unmodifiable collections (which appears mutable, but is made immutable at
> runtime) is something that has also been considered, but there's been no work
> around this.
field named 'age', as well as a setAge(int newAge) and a getAge().
This class is unquestionably non immutable. For what its worth, setAge
isn't SEF, and neither are const. Insofar as we can stretch SEF and
const from methods to an entire class, such a class is clearly
neither.
But let's try and sneak our way around. Let's remove the field
entirely. Now we have a seemingly immutable class; it has no fields!
However, now we have a broken setAge and getAge() method. We can fix
it, though:
public class Fakeout {
private static final Map<Fakeout, Integer> map =
MapMaker.weakKeys().makeMap();
public Fakeout() { map.put(this, 0); }
public void setAge(int age) { map.put(this, age); }
public int getAge() { return map.get(age); }
}
This class is indistinguishable in operation from our original, and
due to the weak keys it won't even leak memory. It will eat up a lot
more memory, due to that map, but that memory will be linear to how
many instances of Fakeout you have. If an instance is garbage
collected, its entry will also disappear from the map. Nevertheless an
immutability checker is rather unlikely to detect this situation, I
don't really know how you could do it. I freely grant that doing the
above is really stupid, but the fact that I can fake out an
immutability detector without any warning bells going off is somewhat
concerning, especially as there are a number of other ways to set up
such a situation that isn't as obviously contrived as the above
example.
This example is the primary driver for my opinion that in the JVM
model, 'immutability', as a concept, is not particularly useful, at
least not without combining it with methods that are all SEF and
preferably const. Or, well, useful, yes, but only as a hint.
>
> Is this what is the intention of C++'s const keyword?
and other functional languages. Functional Languages / languages
focussed on recursion appear at least on the surface insanely
inefficient, having to repeatedly dig through enormous stacks to do
their work. They solve this seeming inefficiency by enforcing const
nature against as many functions as possible so they can store tuples
of "If you call method X with parameters A, B, and C, I already know
the result is D, and it can't change so I don't need to calculate it
again". See memoization on wikipedia. That's a side benefit of playing
around with const / memoization on the JVM: It should make interacting
with e.g. clojure more smooth.
>
> There is an aspect of the tool that recognises the complications around
> subtyping. For instance, a class X not declared final can still be considered
> immutable. However, class Y, a client of the non-final class X, will reflect the
> possibility that the X could be subclassed and screw up Y's mutability status.
>
own methods, some state may still change. Nevertheless great to hear
there's a distinction.
>
> Rightly or wrongly, I took the path that said j.l.Object is immutable, and the
> wait/notify methods are exempt as they: hide below the native surface level;
> are low level synchronisation details that don't really represent state as a
> developer writing their own classes would see it; and... the thought of
> figuring out what it means for every class if j.l.Object is mutable didn't
> really bear thinking about :-) NB: this is the same approach taken for
> concepts such as reflection and AOP - basically my head's in the sand.
issue is definitely the right call or any tool or introspection is
dead on arrival. It does, however, highlight @Synchronized's
brilliance; never ever use any object as lock, ever, unless it's
dedicated to just the job of locking on it (which means in 99.99% of
all cases, if you lock on something whose .getClass() does not return
Object.class, you're doing it wrong). This is unfortunately not common
practice amongst the java world, but nevertheless stating that a class
or object is immutable / SEF / const except for monitor behaviour is
still useful, so either way, good call.
You can change final variables with reflection, heck, you can even
change string constants. Really weird stuff happens, the JVM actively
optimizes based on the presumption that finals don't actually change,
as does other code. Ignoring reflection is the right thing to do :)
> The goal as I see it at the moment, is to try to create a tool to provide the
> most use to developers writing immutable classes. Most of the time (AFAICT)
> this generally isn't too difficult, and it mostly screwed up by those errors one
> step removed from a compiler error. My thoughts were that instead of providing
> annotations and the tools to check at compile time, instead integrate with
> unit testing frameworks so that developers can write Assert.isImmutab
> le(MyClass.class); This seems the most natural and easily introduced way to
> bring in automated mutability analysis. This is still just in the
> contemplation state though.
is that it has documentation benefits: The javadoc could up-front
state that: Yes, instances of this class are (intended to be)
immutable.
>
> Certainly the points you raise about SEF-ness are relevant and interesting
> (and I must say I'm probably falling short of properly understanding the
> subtleties atm) but I think they may just be a bit lofty (and the path be
> littered with dragons) for what I'm trying to achieve.
I've explored only a tiny bit of it so far.
> To summarise it as
> succintly as I can, it's not even close to the level of sophistication you're
> talking about :-)
stuff. You win.
Graham Allan
>
> To: Project Lombok <project...@googlegroups.com>
>
This is something that has come to light in the issue comments I linked to in
the last mail. As the tool is now, if your map variable had been an instance
field, the class would change from being immutable to mutable. Either because
the result of makeMap() is a mutable type, or because the Map instance
assigned to the field is an abstract type, so we can't be sure that the
concrete type at runtime will be immutable.
However, declaring it static at the moment (in code that's not even committed
yet ;-P) means it is found to be immutable. Although it gets the result you're
after in this case, it gets it through ignorance. What I expect will need to
be done, is that method bodies need to be analysed to find if their result
relies on mutable state, instance or static. Huge complexity in that, and even
if I could write the code that detected this perfectly, I reckon the time
spent analysing code would go up by an order of magnitude.
> This example is the primary driver for my opinion that in the JVM
> model, 'immutability', as a concept, is not particularly useful, at
> least not without combining it with methods that are all SEF and
> preferably const. Or, well, useful, yes, but only as a hint.
>
> > Is this what is the intention of C++'s const keyword?
>
> I don't think so. It is however a big concept in haskell and clojure,
> and other functional languages. Functional Languages / languages
> focussed on recursion appear at least on the surface insanely
> inefficient, having to repeatedly dig through enormous stacks to do
> their work. They solve this seeming inefficiency by enforcing const
> nature against as many functions as possible so they can store tuples
> of "If you call method X with parameters A, B, and C, I already know
> the result is D, and it can't change so I don't need to calculate it
> again". See memoization on wikipedia. That's a side benefit of playing
> around with const / memoization on the JVM: It should make interacting
> with e.g. clojure more smooth.
>
I think I see what you mean, and I'm wondering if java.lang.String's
hashCode() method could be considered an example of this. The int value
returned from hashCode() is calculated once (lazily) and always returned: i.e.
hashCode() is method X with result D, but no parameters. Currently the tool
finds String mutable because of this, but it's definitely something that needs
to be addressed. Again it's an issue of analysing the control and data flow of
methods that raises complexity and the time to execute, but it's something
that I will be considering.
> > There is an aspect of the tool that recognises the complications around
> > subtyping. For instance, a class X not declared final can still be
> > considered immutable. However, class Y, a client of the non-final class
> > X, will reflect the possibility that the X could be subclassed and screw
> > up Y's mutability status.
>
> though, with all methods virtual, if a method in X calls one of its
> own methods, some state may still change. Nevertheless great to hear
> there's a distinction.
>
In that scenario, X would still be immutable, but a subclass Y, overriding
internal methods to change state, would not be. The fact an instance of Y is
mutable doesn't mean that an instance of X becomes mutable - if you construct
X directly, you're still constructing an immutable instance. However, this
goes back to what I was saying about clients receiving instances of X - we
can't be sure that defective subclass isn't passed instead.
> [snip]
>
> > The goal as I see it at the moment, is to try to create a tool to provide
> > the most use to developers writing immutable classes. Most of the time
> > (AFAICT) this generally isn't too difficult, and it mostly screwed up by
> > those errors one step removed from a compiler error. My thoughts were
> > that instead of providing annotations and the tools to check at compile
> > time, instead integrate with unit testing frameworks so that developers
> > can write Assert.isImmutab le(MyClass.class); This seems the most
> > natural and easily introduced way to bring in automated mutability
> > analysis. This is still just in the contemplation state though.
>
> That does sound rather useful. Though, the advantage of an annotation
> is that it has documentation benefits: The javadoc could up-front
> state that: Yes, instances of this class are (intended to be)
> immutable.
>
JSR-305 would have been handy here, but it seems to be inactive. If developers
started with putting @Immutable on their type declaration, it would have been
nice, not just for documentation, but for this to automatically trigger a
mutability analysis in their unit-testing framework of choice.
Thanks again for your comments,
Graham
Reinier Zwitserloot
glad JSR305 is dead. The _IDEA_ is great, someone should try to
restart a new JSR with the same ideas but a proper implementation of
them.
For example, non-null types in JSR-305 speak:
@Nullable(NullableType.NEVER) String x;
Just FYI - I used to be a JSR305 supporter too until I actually saw
what the annotations were.
On Jul 4, 2:58 pm, Graham Allan <grundlefl...@googlemail.com> wrote:
> > From: Reinier Zwitserloot <reini...@gmail.com>
>
Graham Allan
I think the problem with that @Nullable stuff, and a couple of others too, is that they seem to be written to only make the tool writer's jobs easier. Which is fine, except they don't seem to consider how difficult it will be for developers to introduce and maintain.
The set of annotations taken from Java Concurrency in Practice (@Immutable, @ThreadSafe) looked pretty reasonable. Which leads on to another question, which yourself and others on this group are probably well positioned to answer (hope you don't mind me asking):
a) can annotations be interpreted without caring, or needing to know about the specific annotation type? I.e. can is say `if("Immutable".equals(someAnnotation.hypotheticalGetNameMethod())) { .... }` when inspecting the bytecode? Whether that's javax.annotation.concurrent.Immutable or some.other.Immutable. (I'm going to check the ASM library, but I thought one of you guys may know off hand).
b) would this specific example be a reasonable thing to, if I were to introduce an annotation processor that could emit errors if a class had @Immutable, and failed Mutability Detector's analysis?
Regards,
Graham
> From: Reinier Zwitserloot <rein...@gmail.com>
>
> To: Project Lombok <project...@googlegroups.com>
>
> About half of the annotations on JSR-305 are ridiculous. I'm very very
> glad JSR305 is dead. The IDEA is great, someone should try to
Reinier Zwitserloot
--Reinier Zwitserloot