Mike,
On 12/21/10 09:38 PM, Mike Duigou wrote:
Thanks. That's an important clarification to include. Here's the revised text:
*
*<p>Care must also be exercised when using collections that do not permit
* calling the {...@code contains} method with a {...@code null} value. If
either
* collection does not permit {...@code contains(null)} then both
collections
* must not contain {...@code null} values.
*
and the @throws text:
* @throws NullPointerException if either collection is {...@code null}.
May
* also be thrown if one collection contains a {...@code null} value and
the
* other collection does not permit {...@code contains(null)}.
My concern with this revised wording is that you are now specifying that
the implementation must use contains() ( and not be implemented using a
different strategy ). I guess an alternative implementation is unlikely,
but this does appear overly restricting.
I wonder if its really necessary to add text to the NPE. A cautionary
note may be sufficient. We could also throw ClassCastException, but
there is no mention of it in the spec.
Sorry for being a pain about this, I'm just concerned with adding overly
restricting spec.
Have we thought about catching/swallowing these exceptions?
-Chris.
Mike
On Dec 21 2010, at 13:15 , Eamonn McManus wrote:
I think that is still not quite right. The spec for Collection says that a Collection that does not
support adding null values may or may not support looking them up. So in both places where you say
"does not permit null values" I think you should probably say "does not permit
looking up null values".
Éamonn
On 21/12/2010 20:35, Mike Duigou wrote:
On Dec 21 2010, at 02:43 , Chris Hegarty wrote:
On 12/21/10 02:24 AM, David Holmes wrote:
Functionality looks okay to me.
I think those spec/doc clarifications may need to go through CCC though.
I agree with David, a CCC request should be filed for the spec changes. We
should agree the spec changes on this mailing list before proposing them.
I understand where you're coming from with this spec change, but I think the
additional text may be too restricting.
"@throws NullPointerException ......... or if one collection
contains {...@code null} and the other collection does not permit
{...@code null} values."
For example, the following would be required to throw NPE ( but I don't believe
your impl does):
Set set = new HashSet();
set.add(null);
PriorityQueue pq = new PriorityQueue();
Collections.disjoint(set, pq);
I think we may have to be a little more relaxed here, maybe just a cautionary note,
"it may happen"???
You are correct that it's not guaranteed that NPE will be thrown. Here's the
amended text for the main javadoc:
*<p>Care must also be exercised when using a mix of collections that
* permit {...@code null} values and those that do not. If either
* collection does not permit {...@code null} values then {...@code null}
must
* not be a value in either collection.
*
and this is the revised @throw NullPointerException:
* @throws NullPointerException if either collection is {...@code null}.
May
* also be thrown if one collection contains a {...@code null} value and
the
* other collection does not permit {...@code null} values.
Note that the descriptive paragraph says "must not" because we don't commit to which
collection is used for contains() and the @throw says "may" because, per your example, if
the collection not permitting null is used for iteration then NPE will not be thrown.
Mike
