Code Review Request for Bug #4802647

David Holmes david.holmes at
Mon Nov 21 02:19:03 UTC 2011

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) {


> Thanks.

More information about the core-libs-dev mailing list