About @Nonnull

135 views
Skip to first unread message

Fabrizio Giudici

unread,
Jan 15, 2010, 8:44:21 PM1/15/10
to project...@googlegroups.com
Hi to everybody and thanks for making Lombok available.

I've some points about the @Nonnull handling, that in my opinion is not
completely flexible. If I understand well, putting @Nonnull together
with @Getter @Setter will make lombok copying and enforcing it on the
setters. Adapting an example from the Lombok docs:

@Getter @Setter @NonNull
private String name;

Equivalent Java source code:

@NonNull
private String name;

public Family(@Nonnull name) {
if (name == null) throw new java.lang.NullPointerException("name");
this.name = name;
}

@NonNull
public String getName() {
return name;
}

public void setName(@NonNull final String name) {
if (v == null) throw new java.lang.NullPointerException("name");
this.name = name;
}

Indeed, I could choose not to write the above code, but rather:

@CheckForNull
private String name;

public Family(@Nonnull name) {
if (name == null) throw new java.lang.NullPointerException("name");
this.name = name;
}

@CheckForNull
public String getName() {
return name;
}

public void setName(@NonNull final String name) {
if (v == null) throw new java.lang.NullPointerException("name");
this.name = name;
}


If the name field is not initialized to non-null in the constructor,
this means that it can be null by default and I do want @CheckForNull so
FindBugs will make sure that I safely dereference it. On the other hand,
this could be a property that somebody could set by calling setName(),
and in that case I won't accept any null value, hence the @Nonnull in
the method parameter. So, to me it sounds as it would be better if
Lombok ignored any @Nonnull annotation on the field for the @Setter, and
an attribute of the annotation is used. That is, my example of code
would be generated by something such as:

@CheckForNull @Getter @Setter(annotations={Nonnull.class})
private String name;


Also, it would be nice if Lombok generated an assert instead of throwing
the NPE if the setter is private.

As a minor note - because I reckon that I'm probably the only one to do
that - it would be nice if one could pick the exception to throw; when
enforcing parameter preconditions, I prefer to throw
IllegalArgumentException rather than NPE.


Final question for now: talking about new features, is it in the scope
of Lombok to enhance code of existing methods, or is it limited to
generate brand new methods? Because it would be nice e.g. to add the
null check in *any* method manually written by the programmer where
there's an argument annotated with @Nonnull.

Thanks!

--
Fabrizio Giudici - Java Architect, Project Manager
Tidalwave s.a.s. - "We make Java work. Everywhere."
weblogs.java.net/blog/fabriziogiudici - www.tidalwave.it/people
Fabrizio...@tidalwave.it

Reinier Zwitserloot

unread,
Jan 16, 2010, 3:23:37 AM1/16/10
to Project Lombok
We're not 100% happy yet with the flexibility of @Data and friends
especially concerning generating constructors and acting specially in
response to certain commonly understood annotations. @NonNull is not
the only annotation out there.

However, creating a hybrid system where the setter requires you not to
set to null, but other sources might so the getter needs a
@CheckForNull is not something we are planning to support as a
specific case. Perhaps at some point we come up with a system where
you get the flexibility to configure things so you can tell lombok to
make this happen, but that's the only way I see lombok supporting this
pattern. That's because it's clearly a non-standard somewhat screwed
up pattern. No real life project ever gets to 0 WTFs / kloc - I'm not
saying the above doesn't or shouldn't happen or even that it should
be treated as a code smell. Merely saying that it should be rare - and
therefore its not something lombok needs to handle. Lombok handles
common cases. Just write it out.


Modifying existing methods is well within scope for lombok. I've been
toying with the idea of using lombok to generate common sanity checks
before, but given the massive array of different needs out there, it
would have to be pluggable somehow. Good idea about how to make such a
thing are always welcome.

On Jan 16, 2:44 am, Fabrizio Giudici <fabrizio.giud...@tidalwave.it>
wrote:

> Fabrizio.Giud...@tidalwave.it

Fabrizio Giudici

unread,
Jan 16, 2010, 7:38:42 AM1/16/10
to project...@googlegroups.com
Reinier Zwitserloot wrote:
>
> Modifying existing methods is well within scope for lombok. I've been
> toying with the idea of using lombok to generate common sanity checks
> before, but given the massive array of different needs out there, it
> would have to be pluggable somehow. Good idea about how to make such a
> thing are always welcome.
>
Absolutely. Another thing that would help me a lot would be generating
logging information at the entering of a method, but of course there are
not only many different logging facilities, but also different styles of
logging.

Anyway, what's the policy for discussing things? I've quickly browsed
the mailing list archive and apparently didn't find info about that -
maybe I did it too quickly. Do you prefer first discussing in the
mailing list or entering RFEs in the issue tracker?

--
Fabrizio Giudici - Java Architect, Project Manager
Tidalwave s.a.s. - "We make Java work. Everywhere."
weblogs.java.net/blog/fabriziogiudici - www.tidalwave.it/people

Fabrizio...@tidalwave.it

Reinier Zwitserloot

unread,
Jan 16, 2010, 4:10:05 PM1/16/10
to Project Lombok
Morbok is a lombok extension that actually does some logging bits,
though not exactly method entry/exit. Technically AOP can do that, but
AOP and lombok don't work very well together unfortunately.

This group is an easier forum for back-and-forth, so we prefer
(potential) RFEs to be discussed here first, fleshed out into
something concrete, pros and cons covered, etc, and then moved over as
an RFE issue with a link back to the thread.

On Jan 16, 1:38 pm, Fabrizio Giudici <fabrizio.giud...@tidalwave.it>
wrote:

> Fabrizio.Giud...@tidalwave.it

Reply all
Reply to author
Forward
0 new messages