akshattandon / projectlombok

Automatically exported from code.google.com/p/projectlombok
0 stars 0 forks source link

@Getter Boolean isWorking produces isWorking() instead of getIsWorking() #148

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
The webpage http://projectlombok.org/features/GetterSetter.html says "Any 
variation on boolean will not result in using the is prefix instead of the get 
prefix; for example, returning java.lang.Boolean results in a get prefix, not 
an is prefix."

However, I annotate a class:
@Getter @Setter 
public class Test { 
   private Boolean isWorking;
}

The produced getter with 0.9.3 is isWorking(); instead of setIsWorking();

Original issue reported on code.google.com by bobth...@gmail.com on 7 Sep 2010 at 1:43

GoogleCodeExporter commented 9 years ago
Heh, by centralizing the detection of booleans, this part got updated too.

I'm guessing you'd rather see that the method is indeed called isWorking(), and 
that we should update the documentation. Am I missing a good reason to go with 
getIsWorking() instead?

Original comment by reini...@gmail.com on 7 Sep 2010 at 11:53

GoogleCodeExporter commented 9 years ago
One reason to go with getIsWorking() is to keep the code it generates 
compatible with the code that Eclipse generates-- which would make switching 
easier for already existing code.

Original comment by bobth...@gmail.com on 7 Sep 2010 at 3:57

GoogleCodeExporter commented 9 years ago
If the field name starts with "is" and then an uppercase letter (or effective 
word boundary equivalent), I think the getter should start with "get".

In any event, the text on the GetterSetter page should be updated to reflect 
what actually happens today.

Original comment by jcpet...@gmail.com on 8 Dec 2010 at 7:31

GoogleCodeExporter commented 9 years ago
So, 'foo' becomes 'isFoo()', but 'isFoo' becomes 'getIsFoo()'? That makes no 
sense at all to me.

it's a shame the beanspec had to be so convoluted in regards to boolean 
handling, but the notion that eclipse doesn't scan java.lang.Boolean as boolean 
gives away that this is uncharted territory for not just lombok, which makes me 
feel better.

I'm getting the sense this is one of those coinflip moments: It really doesn't 
matter what we do, we'll piss off half the community and make the other half 
happy (and by happy coincidence, actually 95% won't care, this is such a corner 
case). As lombok currently already does the prefix thing I'm tempted to stick 
with it and update the docs to match actual behaviour.

Though, that opens the door to a second issue: We already give 'get', 'is' and 
'has' special treatment. If you know that, principle of least surprise dictates 
'can' should be in that list of prefixes too. It isn't now. Still tempted to 
stick with what we do now just because that gives the least backwards 
compatibility headaches, even if its not the most consistent treatment, and not 
what the docs currently say.

Original comment by reini...@gmail.com on 9 Dec 2010 at 6:31

GoogleCodeExporter commented 9 years ago
I don't think it is just Eclipse, but it is also NetBeans, Hibernate, and other 
code generation tools that work different than how Lombok works currently.  
Also, I think following the bean spec yields boolean isFoo() and Boolean 
getIsFoo().  As far as least surprise goes, it would be nice if Lombok was 
consistent with those.

Also, a Boolean is not a boolean.  A Boolean is an Object, and "least surprise" 
says that all Objects should be treated the same.   The exception to the "least 
surprise" should only be for primitive booleans. I know the line between 
boolean and Boolean is a bit blurred these days because of auto-boxing, but a 
boolean is still different than Boolean.

http://netbeans.org/bugzilla/show_bug.cgi?id=160531

Original comment by bobth...@gmail.com on 9 Dec 2010 at 1:19

GoogleCodeExporter commented 9 years ago
Do you have a link to the relevant part of the beans spec? If it is 
non-ambiguous on this + the fact that everyone else apparently rolls with 
getIs, would be compelling evidence.

I assume the name becomes getIsFoo() for no special reason, just applying the 
standard 'get in front, titlecase first char of field name' algorithm to a 
field named 'isFoo'.

What does netbeans and company do when you write 'isFoo' as a field? isIsFoo()?

Original comment by reini...@gmail.com on 9 Dec 2010 at 1:23

GoogleCodeExporter commented 9 years ago
Er, whoops, that link IS the proof. Still interested in what netbeans does for:

private boolean isFoo;

Roel: I guess we'll break backwards compatibility and remove boolean treatment 
for Booleans?

NB: Le sigh. Another crazy corner case that's going to make eliminating 
primitives from java (the language) needlessly difficult.

Original comment by reini...@gmail.com on 9 Dec 2010 at 1:26

GoogleCodeExporter commented 9 years ago

Ah, maybe we can find a happy solution: implement the getter both ways.  The 
spec says that is allowed:  "This “is<PropertyName>” method may be provided 
instead of a “get<PropertyName>” method, or it may be provided in addition 
to a “get<PropertyName>” method." (section 8.3.2 of JavaBeans 1.01-A)

As for the example "boolean isFoo;"-- I can't say for Netbeans, but Eclipse 
generates isFoo() and setFoo(), and for "Boolean isBar;", Eclipse generates 
getIsBar() and setIsBar().  There is nothing in the spec that says that you 
can't assume the property name for "Boolean isBar;" is "bar"-- if that was the 
case, then the required methods would be getBar() and setBar().  Because 
getBar() does not do a great job of showing that it is a BOOL property, it 
seems that assuming the property name is "isBar" might  be preferred.

The JavaBeans spec don't specify any tie between the field name and the 
property name-- so I suppose Eclipse takes the liberty of assuming that when 
you have a field boolean isFoo, that you really want a property name of "foo", 
but when you have a field Boolean isBar, that you really want a property name 
of "isBar".  Things get muddy when you try to cover every option.  I mapped out 
every option below, but I suggest maybe just include the "required by spec" and 
"optional by spec".  It wouldn't hurt to include some of the other methods if 
they really appeal to you.

/////
boolean foo; // assume property name "foo"
void setFoo(boolean b); // required by spec
boolean isFoo();  // required by spec if getFoo() is not implemented.  If this 
and getFoo() both are implemented, APIs should prefer to call this method.
boolean getFoo(); // required by spec if isFoo() is not implemented.
/////
Boolean bar; // assume property name "bar"
void setBar(Boolean b); // required by spec
Boolean getBar(); //required by spec
Boolean isBar(); // not mentioned in spec
/////
boolean isWorking; // assume property name "working"
void setWorking(boolean b);  // required by spec
boolean isWorking();  // required by spec if getWorking() is not implemented.  
If this and getWorking() both are implemented, APIs should prefer to call this 
method.
boolean getWorking();  // required by spec if isWorking() is not implemented.
// Also, to be interchangeable with "Boolean isWorking;", the following would 
methods would be needed that assume a property name of "isWorking":
void setIsWorking(boolean b);  // not mentioned in spec
boolean getIsWorking();  // not mentioned in spec
/////
Boolean isNotWorking; // assume property name "isNotWorking"
void setIsNotWorking(Boolean b);  // required by spec
Boolean getIsNotWorking();  // required by spec
Boolean isIsNotWorking(); // not mentioned in spec
// Also, to be interchangeable with "boolean isNotWorking;", the following 
would methods would be needed that assume a property name of "notWorking":
void setNotWorking(Boolean b);  // not mentioned in spec
Boolean isNotWorking(); // not mentioned in spec
/////

https://cds.sun.com/is-bin/INTERSHOP.enfinity/WFS/CDS-CDS_Developer-Site/en_US/-
/USD/ViewProductDetail-Start?ProductRef=7224-javabeans-1.01-fr-spec-oth-JSpec@CD
S-CDS_Developer

Original comment by bobth...@gmail.com on 9 Dec 2010 at 3:25

GoogleCodeExporter commented 9 years ago
Great work, bob. Thanks. The problem with your last one (adding 'setIsWorking 
and 'setWorking' both) is that this effectively creates 2 properties, one named 
'isWorking' and the other named 'working'. That's a bit much. Other than that, 
the answer seems clear:

(A) Boolean is like any other object - we strip 'get' off the front if the 
field name is getX, with X being a non-lowercase letter, otherwise we make no 
changes, and the getter name is formed by prefixing with 'get' and titlecasing 
the (stripped) field name if the (stripped) field name starts with a lowercase 
letter.

(B) for booleans we do something similar, except 'is', and 'has', in addition 
to 'get', count as field name prefixes that will be stripped out. The getter 
name is generated by prefixing the title-cased stripped field name with 'is' 
instead of 'get'.

This is mostly similar to what happens in lombok today. The changes:

A) Booleans lose their special treatment. They get prefixed with 'is' today.

B) a field named 'isFoo' currently gives 'setIsFoo' for setter. We'll be 
changing that to 'setFoo'.

Original comment by reini...@gmail.com on 3 Jan 2011 at 4:34

GoogleCodeExporter commented 9 years ago
this is really a problem for me. my colleague added a field which named 
is_Management_Acct then spring doesn't init the field Object, then null point 
exception come out. here is the code :
 @Getter @Setter ComboBox is_Management_Acct;
 this.is_Management_Acct.setValue ( responseHandler.getWriter ( ) , xmlContainer.getField ( "/RETURN/CPF" ) );

Original comment by lpin...@gmail.com on 20 Jan 2011 at 12:34

GoogleCodeExporter commented 9 years ago
To commenter #10 (lpin...@gmail.com): That doesn't sound related to this 
problem, or in fact to anything lombok did. As ComboBox is not a Boolean nor a 
boolean, getIs and setIs are generated as usual.

This will be fixed before 0.10 is out the door as this is a backwards 
incompatible change (admittedly in rare circumstances) which means we need to 
do a major version bump. Might as well take care of it with this one.

Original comment by reini...@gmail.com on 28 Jan 2011 at 6:49

GoogleCodeExporter commented 9 years ago
Fixed in c98cec7d2ddceddcc0f127185912be4f826a6caa and 
30b63deaff82128ca8c3e7b59884b4fc2caf929c

Will be in 0.10.0 final.

lpin...@gmail.com: This should also fix your problem.

Original comment by reini...@gmail.com on 7 Feb 2011 at 8:40