mkodekar / guava-libraries

Automatically exported from code.google.com/p/guava-libraries
Apache License 2.0
0 stars 0 forks source link

Improve Joiner#join documentation and exception message for null behavior #1848

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
I'm not sure whether it is an actual bug so I formulated the subject as a 
question, but I think that if some instances of Joiner (namely those not 
generated from skipNulls() or useForNull(String)) don't support null arguments 
and explicitly throw an NPE the parameters of the Joiner#join() method 
shouldn't be annotated @Nullable.

I also think having a subtype of Joiner (something like NullSupportedJoiner) 
just for this wouldn't make sense, so if this is done to prevent unnecessary 
static analysis warnings on Joiner#join() method calls with possibly null 
parameter values, I would suggest that it at least is added to the 
documentation of the join() method (that the Joiner instance needs to support 
null values for the parameters to be nullable).
It IS explained in the javadoc of the class but it's easy to miss when just 
looking at the method documentation (e.g. from an IDE).

Throwing a more explicit error (e.g. adding a message about "skipNulls()" call 
requirement to the thrown NPE) may also be a nice way to document this 
requirement.

Let me know if I misunderstood something.

Original issue reported on code.google.com by legrand....@gmail.com on 15 Sep 2014 at 7:21

GoogleCodeExporter commented 9 years ago
I think we take the position that a parameter has to be @Nullable if *any* 
implementation of the type permits nulls there. This is certainly not ideal. 
(It's also not ideal that I'm not 100% sure that I have our policy right.) For 
general despair around the issue of nullability annotations, see issue 1812.

For this specific case, we can be more optimistic. We can improve the error 
message and the doc. I'm modifying this bug's title to focus on that.

Original comment by cpov...@google.com on 18 Sep 2014 at 10:00

GoogleCodeExporter commented 9 years ago
This issue has been migrated to GitHub.

It can be found at https://github.com/google/guava/issues/<issue id>

Original comment by cgdecker@google.com on 1 Nov 2014 at 4:08

GoogleCodeExporter commented 9 years ago

Original comment by cgdecker@google.com on 1 Nov 2014 at 4:17

GoogleCodeExporter commented 9 years ago

Original comment by cgdecker@google.com on 3 Nov 2014 at 9:07