Re: RFR : CR8004015 : Add parent interfaces and default methods to basic functional interfaces
On Dec 16 2012, at 20:18 , David Holmes wrote: On 15/12/2012 4:58 AM, Mike Duigou wrote: On Dec 13 2012, at 22:28 , David Holmes wrote: I have added @throws NPE for a number of the default methods. We won't be including @throws NPE in all cases where null is disallowed because when the @throws NPE is declared the API is required to throw NPE in that circumstance. So for cases where the NPE is naturally thrown or that aren't performance sensitive we will likely add @throws NPE declarations but for performance sensitive methods we won't be adding explicit null checks to match a @throws NPE specification. There's a tradeoff here in some cases. Please feel free to quibble about specific cases as they occur. :-) That doesn't make sense to me. The throwing of the NPE is intended to be part of the specification not an implementation choice. Further @param foo non-null, is just as binding on implementations as @throws NPE if foo is null. ??? An @param foo non-null by itself is not considered normative because it doesn't document what happens when null is passed. So it ends up being advisory only. An @throws NPE is considered normative and the implementation must throw in all circumstances described. Aside: that is an interesting interpretation but from whence does it come? From the TCK/JCK team. In communications with them to clarify the specification It is non-normative by definition because it is incomplete? Yes. If a constraint is specified then it is only testable to the extent that the outcome of is also described. Or is it just non-normative because it is an @param tag? No. (Please bear with the step-by-step nature of the following sample, it's incremental pace is not meant to be insulting--it's a write-up for a general FAQ). If I have a method: But once you add the @throws the advisory of the @param is redundant. Hence to me it is an either/or situation. It does seem redundant to me but not entirely useless. I would prefer to not have to check the @throws declaration to know what the valid range is for a parameter. My tendency is to always try to consolidate all information about something in one place and if that's not practical then allow for some duplication. An example recently was information about the ForkJoin common pool. I prefer to see all the characteristics of the common pool described on the documentation for commonPool() method rather than disparately on shutdown(), etc. The advantage being that collecting it into one place allows be to completely understand (OK, within limits) the characteristics of common pool. Without the consolidation I may not even be able to find all of the documentation which references the common pool. There does seem to be some value though to duplicating the information to other places where it might be relevant. It's perhaps worthwhile to note in shutdown() that it is ignored for the common pool. Further the advisory, being advisory, is useless in my opinion, so not something to use regardless. The advisories do still represent good advice for what to pass. Mike
Re: RFR : CR8004015 : Add parent interfaces and default methods to basic functional interfaces
On 15/12/2012 4:58 AM, Mike Duigou wrote:
On Dec 13 2012, at 22:28 , David Holmes wrote:
I have added @throws NPE for a number of the default methods. We won't be including
@throws NPE in all cases where null is disallowed because when the @throws NPE is
declared the API is required to throw NPE in that circumstance. So for cases where the
NPE is naturally thrown or that aren't performance sensitive we will likely
add @throws NPE declarations but for performance sensitive methods we won't be adding
explicit null checks to match a @throws NPE specification. There's a tradeoff here in
some cases. Please feel free to quibble about specific cases as they occur. :-)
That doesn't make sense to me. The throwing of the NPE is intended to be part
of the specification not an implementation choice. Further @param foo non-null,
is just as binding on implementations as @throws NPE if foo is null. ???
An @param foo non-null by itself is not considered normative because it doesn't
document what happens when null is passed. So it ends up being advisory only. An @throws
NPE is considered normative and the implementation must throw in all circumstances described.
Aside: that is an interesting interpretation but from whence does it
come? It is non-normative by definition because it is incomplete? Or is
it just non-normative because it is an @param tag?
(Please bear with the step-by-step nature of the following sample, it's
incremental pace is not meant to be insulting--it's a write-up for a general
FAQ). If I have a method:
But once you add the @throws the advisory of the @param is redundant.
Hence to me it is an either/or situation. Further the advisory, being
advisory, is useless in my opinion, so not something to use regardless.
David
-
/**
* If the object is visible then write it's string form to the provided
PrintStream.
*
* @param bar destination for display
/
public void display(PrintStream bar) {
if(visible) {
bar.write(toString());
}
}
This implementation isn't going to work well if bar is ever null. So I document that in
the @param bar:
/**
* If the object is visible then write it's string form to the provided
PrintStream.
*
* @param bar destination for display, must be non-null
/
public void display(PrintStream bar) {
if(visible) {
bar.write(toString());
}
}
The @param bar documentation now says that it must be non-null but doesn't explain what
happens if null is passed. Having documented that null shouldn't be passed is helpful but not as
helpful as it could be. To specify what happens I must add @throws NPE:
/**
* If the object is visible then write it's string form to the provided
PrintStream.
*
* @param bar destination for display, must be non-null
* @throws NullPointerException if bar is null
/
public void display(PrintStream bar) {
if(visible) {
bar.write(toString());
}
}
This implementation would superficially seem to conform to the contract described in the javadoc.
However, when the if(visible) conditional is false then no NPE will be thrown. Contract
broken. It is required to add an explicit null check on bar ie.
/**
* If the object is visible then write it's string form to the provided
PrintStream.
*
* @param bar destination for display, must be non-null
* @throws NullPointerException if bar is null
/
public void display(PrintStream bar) {
Objects.requireNonNull(bar);
if(visible) {
bar.write(toString());
}
}
Adding the Objects.requireNonNull ensures that the NPE is thrown in all
appropriate cases. There is also another alternative:
/**
* If the object is visible then write it's string form to the provided
PrintStream.
*
* @param bar destination for display, must be non-null
* @throws NullPointerException if bar is null and the component is visible.
/
public void display(PrintStream bar) {
if(visible) {
bar.write(toString());
}
}
The conditions in which NPE are thrown are amended so that throwing is only
required if the component is visible. Documenting the conditions could quickly
become complicated if display was a more involved method. At some point it
becomes easier to just add an explicit check. It can also be useful to add
explicit NPE checks as pre-conditions before a multi-stage operation where a
late stage NPE might corrupt the object state.
In a very few cases an explicit null check might add too much overhead to a performance sensitive method and
writing an accurate @throws NPE isn't possible (perhaps because of delegation) then the
@throws NPE should be removed and only the advisory @param bar ... must be non-null
will have to suffice.
Mike
I think defining the NPE via the @param and @throws is a lose-lose situation:
! * @param left {@inheritDoc}, must be non-null
! * @param right {@inheritDoc}, must be non-null
! * @return {@inheritDoc}, always non-null
! * @throws NullPointerException if {@code left} or {@code right} is null
You only need one
Re: RFR : CR8004015 : Add parent interfaces and default methods to basic functional interfaces
On Dec 13 2012, at 22:28 , David Holmes wrote:
I have added @throws NPE for a number of the default methods. We won't be
including @throws NPE in all cases where null is disallowed because when the
@throws NPE is declared the API is required to throw NPE in that
circumstance. So for cases where the NPE is naturally thrown or that
aren't performance sensitive we will likely add @throws NPE declarations but
for performance sensitive methods we won't be adding explicit null checks to
match a @throws NPE specification. There's a tradeoff here in some cases.
Please feel free to quibble about specific cases as they occur. :-)
That doesn't make sense to me. The throwing of the NPE is intended to be part
of the specification not an implementation choice. Further @param foo
non-null, is just as binding on implementations as @throws NPE if foo is
null. ???
An @param foo non-null by itself is not considered normative because it
doesn't document what happens when null is passed. So it ends up being advisory
only. An @throws NPE is considered normative and the implementation must
throw in all circumstances described.
(Please bear with the step-by-step nature of the following sample, it's
incremental pace is not meant to be insulting--it's a write-up for a general
FAQ). If I have a method:
/**
* If the object is visible then write it's string form to the provided
PrintStream.
*
* @param bar destination for display
/
public void display(PrintStream bar) {
if(visible) {
bar.write(toString());
}
}
This implementation isn't going to work well if bar is ever null. So I
document that in the @param bar:
/**
* If the object is visible then write it's string form to the provided
PrintStream.
*
* @param bar destination for display, must be non-null
/
public void display(PrintStream bar) {
if(visible) {
bar.write(toString());
}
}
The @param bar documentation now says that it must be non-null but doesn't
explain what happens if null is passed. Having documented that null shouldn't
be passed is helpful but not as helpful as it could be. To specify what happens
I must add @throws NPE:
/**
* If the object is visible then write it's string form to the provided
PrintStream.
*
* @param bar destination for display, must be non-null
* @throws NullPointerException if bar is null
/
public void display(PrintStream bar) {
if(visible) {
bar.write(toString());
}
}
This implementation would superficially seem to conform to the contract
described in the javadoc. However, when the if(visible) conditional is false
then no NPE will be thrown. Contract broken. It is required to add an explicit
null check on bar ie.
/**
* If the object is visible then write it's string form to the provided
PrintStream.
*
* @param bar destination for display, must be non-null
* @throws NullPointerException if bar is null
/
public void display(PrintStream bar) {
Objects.requireNonNull(bar);
if(visible) {
bar.write(toString());
}
}
Adding the Objects.requireNonNull ensures that the NPE is thrown in all
appropriate cases. There is also another alternative:
/**
* If the object is visible then write it's string form to the provided
PrintStream.
*
* @param bar destination for display, must be non-null
* @throws NullPointerException if bar is null and the component is visible.
/
public void display(PrintStream bar) {
if(visible) {
bar.write(toString());
}
}
The conditions in which NPE are thrown are amended so that throwing is only
required if the component is visible. Documenting the conditions could quickly
become complicated if display was a more involved method. At some point it
becomes easier to just add an explicit check. It can also be useful to add
explicit NPE checks as pre-conditions before a multi-stage operation where a
late stage NPE might corrupt the object state.
In a very few cases an explicit null check might add too much overhead to a
performance sensitive method and writing an accurate @throws NPE isn't
possible (perhaps because of delegation) then the @throws NPE should be
removed and only the advisory @param bar ... must be non-null will have to
suffice.
Mike
I think defining the NPE via the @param and @throws is a lose-lose situation:
! * @param left {@inheritDoc}, must be non-null
! * @param right {@inheritDoc}, must be non-null
! * @return {@inheritDoc}, always non-null
! * @throws NullPointerException if {@code left} or {@code right} is null
You only need one convention.
David
-
Mike
RFR : CR8004015 : Add parent interfaces and default methods to basic functional interfaces
Hello all; I have updated the webrev again for hopefully the last time: http://cr.openjdk.java.net/~mduigou/8004015/3/webrev/ http://cr.openjdk.java.net/~mduigou/8004015/3/specdiff/overview-summary.html The implementation now uses Primitive.primitiveValue() ie. Integer.integerValue() rather than a cast. Same bytecode but using the intrinsic function makes it more clear that result is either primitive or NPE and that CCE is not possible. I have added @throws NPE for a number of the default methods. We won't be including @throws NPE in all cases where null is disallowed because when the @throws NPE is declared the API is required to throw NPE in that circumstance. So for cases where the NPE is naturally thrown or that aren't performance sensitive we will likely add @throws NPE declarations but for performance sensitive methods we won't be adding explicit null checks to match a @throws NPE specification. There's a tradeoff here in some cases. Please feel free to quibble about specific cases as they occur. :-) Mike
Re: RFR : CR8004015 : Add parent interfaces and default methods to basic functional interfaces
HI Mike,
On 14/12/2012 3:24 PM, Mike Duigou wrote:
Hello all;
I have updated the webrev again for hopefully the last time:
http://cr.openjdk.java.net/~mduigou/8004015/3/webrev/
http://cr.openjdk.java.net/~mduigou/8004015/3/specdiff/overview-summary.html
The implementation now uses Primitive.primitiveValue() ie.
Integer.integerValue() rather than a cast. Same bytecode but using the
intrinsic function makes it more clear that result is either primitive or NPE
and that CCE is not possible.
I have added @throws NPE for a number of the default methods. We won't be including
@throws NPE in all cases where null is disallowed because when the @throws NPE is
declared the API is required to throw NPE in that circumstance. So for cases where the
NPE is naturally thrown or that aren't performance sensitive we will likely
add @throws NPE declarations but for performance sensitive methods we won't be adding
explicit null checks to match a @throws NPE specification. There's a tradeoff here in
some cases. Please feel free to quibble about specific cases as they occur. :-)
That doesn't make sense to me. The throwing of the NPE is intended to be
part of the specification not an implementation choice. Further @param
foo non-null, is just as binding on implementations as @throws NPE if
foo is null. ???
I think defining the NPE via the @param and @throws is a lose-lose
situation:
! * @param left {@inheritDoc}, must be non-null
! * @param right {@inheritDoc}, must be non-null
! * @return {@inheritDoc}, always non-null
! * @throws NullPointerException if {@code left} or {@code right}
is null
You only need one convention.
David
-
Mike
