RFR 8015008: Primitive iterator over empty sequence, null consumer: forEachRemaining methods do not throw NPE (original) (raw)

Chris Hegarty chris.hegarty at oracle.com
Wed May 29 14:37:57 UTC 2013

Looks fine to me Paul.

On 29/05/2013 12:35, Paul Sandoz wrote:

Hi,

Please review these changes to j.u.PrimitiveIterator to ensure the default forEachRemaining methods consistently throw an NPE when the consumer is null. I almost produced a webrev for this, but i thought it was just about acceptable size-wise and i hope easy to review in textual form. If this is considered impolite, awkward to review etc please say so and i will produce a webrev.

I found it a little difficult to review as it lacked some context around the changes. I found myself having to track down the source and compare line numbers. Not a problem, just that you happen to mention it ;-)

-Chris

Paul. # HG changeset patch # User psandoz # Date 1369825083 -7200 # Node ID 7ded996200218791c885c0aae4c474066101c7bd # Parent bfdc1ed75460c9e6869827cf9acabd4b1a5e9d29 8015008: Primitive iterator over empty sequence, null consumer: forEachRemaining methods do not throw NPE Reviewed-by: diff -r bfdc1ed75460 -r 7ded99620021 src/share/classes/java/util/PrimitiveIterator.java --- a/src/share/classes/java/util/PrimitiveIterator.java Wed May 29 12:58:02 2013 +0200 +++ b/src/share/classes/java/util/PrimitiveIterator.java Wed May 29 12:58:03 2013 +0200 @@ -91,6 +91,7 @@ * @throws NullPointerException if the specified action is null */ default void forEachRemaining(IntConsumer action) { + Objects.requireNonNull(action); while (hasNext()) action.accept(nextInt()); } @@ -123,6 +124,8 @@ forEachRemaining((IntConsumer) action); } else { + // The method reference action::accept is never null + Objects.requireNonNull(action); if (Tripwire.ENABLED) Tripwire.trip(getClass(), "{0} calling PrimitiveIterator.OfInt.forEachRemainingInt(action::accept)"); forEachRemaining((IntConsumer) action::accept); @@ -162,6 +165,7 @@ * @throws NullPointerException if the specified action is null */ default void forEachRemaining(LongConsumer action) { + Objects.requireNonNull(action); while (hasNext()) action.accept(nextLong()); } @@ -194,6 +198,8 @@ forEachRemaining((LongConsumer) action); } else { + // The method reference action::accept is never null + Objects.requireNonNull(action); if (Tripwire.ENABLED) Tripwire.trip(getClass(), "{0} calling PrimitiveIterator.OfLong.forEachRemainingLong(action::accept)"); forEachRemaining((LongConsumer) action::accept); @@ -232,6 +238,7 @@ * @throws NullPointerException if the specified action is null */ default void forEachRemaining(DoubleConsumer action) { + Objects.requireNonNull(action); while (hasNext()) action.accept(nextDouble()); } @@ -265,6 +272,8 @@ forEachRemaining((DoubleConsumer) action); } else { + // The method reference action::accept is never null + Objects.requireNonNull(action); if (Tripwire.ENABLED) Tripwire.trip(getClass(), "{0} calling PrimitiveIterator.OfDouble.forEachRemainingDouble(action::accept)"); forEachRemaining((DoubleConsumer) action::accept); diff -r bfdc1ed75460 -r 7ded99620021 test/java/util/Iterator/PrimitiveIteratorDefaults.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/java/util/Iterator/PrimitiveIteratorDefaults.java Wed May 29 12:58:03 2013 +0200 @@ -0,0 +1,115 @@ +/* + * Copyright (c) 2013, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +import org.testng.annotations.Test; + +import java.util.PrimitiveIterator; +import java.util.function.Consumer; +import java.util.function.DoubleConsumer; +import java.util.function.IntConsumer; +import java.util.function.LongConsumer; + +import static org.testng.Assert.assertNotNull; +import static org.testng.Assert.assertTrue; + +/** + * @test + * @run testng PrimitiveIteratorDefaults + * @summary test default methods on PrimitiveIterator + */ + at Test +public class PrimitiveIteratorDefaults { + + public void testIntForEachRemainingWithNull() { + PrimitiveIterator.OfInt i = new PrimitiveIterator.OfInt() { + @Override + public int nextInt() { + return 0; + } + + @Override + public boolean hasNext() { + return false; + } + }; + + executeAndCatch(() -> i.forEachRemaining((IntConsumer) null)); + executeAndCatch(() -> i.forEachRemaining((Consumer) null)); + } + + public void testLongForEachRemainingWithNull() { + PrimitiveIterator.OfLong i = new PrimitiveIterator.OfLong() { + @Override + public long nextLong() { + return 0; + } + + @Override + public boolean hasNext() { + return false; + } + }; + + executeAndCatch(() -> i.forEachRemaining((LongConsumer) null)); + executeAndCatch(() -> i.forEachRemaining((Consumer) null)); + } + + public void testDoubleForEachRemainingWithNull() { + PrimitiveIterator.OfDouble i = new PrimitiveIterator.OfDouble() { + @Override + public double nextDouble() { + return 0; + } + + @Override + public boolean hasNext() { + return false; + } + }; + + executeAndCatch(() -> i.forEachRemaining((DoubleConsumer) null)); + executeAndCatch(() -> i.forEachRemaining((Consumer) null)); + } + + private void executeAndCatch(Runnable r) { + executeAndCatch(NullPointerException.class, r); + } + + private void executeAndCatch(Class<? extends Exception> expected, Runnable r) { + Exception caught = null; + try { + r.run(); + } + catch (Exception e) { + caught = e; + } + + assertNotNull(caught, + String.format("No Exception was thrown, expected an Exception of %s to be thrown", + expected.getName())); + assertTrue(expected.isInstance(caught), + String.format("Exception thrown %s not an instance of %s", + caught.getClass().getName(), expected.getName())); + } + +}

More information about the core-libs-dev mailing list