Code Review Request for Bug #4802647

Brandon Passanisi brandon.passanisi at
Mon Nov 21 18:29:00 UTC 2011

Thank you for the review David.  I'll make the changes to the test 
program as you have suggested and I will also update the bug report with 
the comments you have given.  I'll then send out an updated webrev.  
Just to double-check, when you mention "But I don't see a way around it" 
in regards to the fix to it sounds like you 
agree to keep the change as-is.  Is this correct?

On 11/20/2011 6:19 PM, David Holmes wrote:
> Hi Brandon,
> On 19/11/2011 11:21 AM, Brandon Passanisi wrote:
>> Hello core-libs-dev at Please review the following patch
>> to fix Bug 4802647 (coll) NullPointerException not thrown by
>> AbstractCollection.retainAll/removeAll: The fix is quite small and I
>> have included a test program, all of which can be found within the
>> following webrev:
> The bug report is a little misleading. The bug only exists when the 
> current collection is empty - in which case we skip most of the method 
> body. Otherwise we will generate the NPE when we call 
> c.contains( The CR should be updated to note this.
> So while your fix is correct, I hate to see good code penalized 
> (explicit null check) to allow bad code to throw exceptions as 
> required. But I don't see a way around it.
> With regard to the test program - it can be simplified somewhat as 
> unexpected exceptions can just be allowed to propagate and failures to 
> throw can be detected inline:
>      public static void main(String[] args) {
>          // Ensure removeAll(null) throws NullPointerException
>          try {
>              (new NewAbstractCollection<>()).removeAll(null);
>              fail("NullPointerException not thrown for 
> removeAll(null).");
>          } catch (NullPointerException expected) {
>          }
>          // Ensure retainAll(null) throws NullPointerException
>          try {
>              (new NewAbstractCollection<>()).retainAll(null);
>              fail("NullPointerException not thrown for 
> retainAll(null).");
>          } catch (NullPointerException expected) {
>          }
>      }
> Cheers,
> David
>> Thanks.

Oracle <>
Brandon Passanisi | Principle Member of Technical Staff

Oracle Java Standards Conformance

Green Oracle <> Oracle is committed to 
developing practices and products that help protect the environment

More information about the core-libs-dev mailing list