CODEWITHSUNDEEP
java
Joker
Joker

Reputation: 11140

Collections.singleton() and forEachRemaining - Java 8

While working on Collections.singleton() I found that it is not working as expteced. If you see the code below after forEachRemaining code is neither throwing any exception nor returning false on itr.hasNext()

From Java Docs of forEachRemaining

Performs the given action for each remaining element until all elements have been processed

The out put of below code is: true,elem and I am expecting false, NoSuchElementException

public class Test {
        public static void main(String[] args) {
        Collection<String> abc = Collections.singleton("elementsItr");
        final Iterator<String> itr = abc.iterator();
        try {
            itr.forEachRemaining((e) -> {
                throw new RuntimeException();
            });
        } catch (RuntimeException e) {

        }
        System.out.println(itr.hasNext());
        System.out.println(itr.next());

    }
}

Please help me understand this behavior.

Upvotes: 4

Views: 2047

Answers (4)

Pallavi Sonal
Pallavi Sonal

Reputation: 3901

This had been reported in OpenJDK with JDK-8166446. The analysis states :

I don't see how this would violate the specification. http://docs.oracle.com/javase/8/docs/api/java/util/Iterator.html#forEachRemaining-java.util.function.Consumer-

""" Performs the given action for each remaining element until all elements have been processed or the action throws an exception. Actions are performed in the order of iteration, if that order is specified. Exceptions thrown by the action are relayed to the caller. """

It doesn't say whether the iterator should be "incremented" or not, if accept() throws an exception.

Some other classes expose the same behavior as singleton: E.g. ArrayList.Iterator has the following code: public void forEachRemaining(Consumer consumer) { ........ } while (i != size && modCount == expectedModCount) { consumer.accept((E) elementData[i++]); } // update once at end of iteration to reduce heap write traffic cursor = i; lastRet = i - 1; ........ }

---------- I think it's not a bug.

However, it may make sense to unify the behavior of different implementations of forEachRemaining() just to make them consistent with the default implementation.

With JDK 9 , this was fixed and if you test with the latest JDK 9-ea , the output is as below:

Is empty: false
Exception in thread "main" java.util.NoSuchElementException
        at java.base/java.util.Collections$1.next(Collections.java:4698)
        at Test.main(Test.java:18)

Upvotes: 2

davidxxx
davidxxx

Reputation: 131496

The iterator used by SingletonSet<E> that is the class used to instance the singleton when Collections.singleton(T) is invoked explains this behavior.
The iterator is provided by the static <E> Iterator<E> Collections.singletonIterator(E e) method.

This has a hasNext flag that pass to false only if the consummer has returned without exception :

static <E> Iterator<E> singletonIterator(final E e) {
   ...   
        @Override
        public void forEachRemaining(Consumer<? super E> action) {
            Objects.requireNonNull(action);
            if (hasNext) {
                action.accept(e);
                hasNext = false;
            }
        }
   ...
}

The documentation of the method doesn't explicit this behavior but we could consider it as a safe check to step forward the iterator only it was correctly consumed.
But it gives however a hint about the exception handling :

Exceptions thrown by the action are relayed to the caller.

So if a exception occurs during forEachRemaining(), there is no guarantee on the state of the iterator as the exception is not caught by forEachRemaining() but forwarded to the caller.

Upvotes: 2

ajb
ajb

Reputation: 31699

Looking at the code: Collections.singleton() returns a SingletonSet. If you call iterator() on a SingletonSet, the resulting iterator is of an anonymous class. The anonymous class overrides forEachRemaining:

public void forEachRemaining(Consumer<? super E> action) {
    Objects.requireNonNull(action);
    if (hasNext) {
        action.accept(e);
        hasNext = false;
    }
}

Since your accept throws an exception, hasNext remains true.

Note that the javadoc doesn't specify what should happen to forEachRemaining if an exception is thrown; thus, it's possible that the next version of the runtime could put hasNext = false above the action.accept(e), leading to a different result. So you can't count on the behavior one way or the other.

Upvotes: 5

JDC
JDC

Reputation: 4385

If you look closely you will see that you get a singleton iterator (Collections#singletonIterator()).

The code of it looks like that:

static <E> Iterator<E> singletonIterator(final E e) {
    return new Iterator<E>() {
        private boolean hasNext = true;
        public boolean hasNext() {
            return hasNext;
        }
        public E next() {
            if (hasNext) {
                hasNext = false;
                return e;
            }
            throw new NoSuchElementException();
        }
        public void remove() {
            throw new UnsupportedOperationException();
        }
        @Override
        public void forEachRemaining(Consumer<? super E> action) {
            Objects.requireNonNull(action);
            if (hasNext) {
                action.accept(e);
                hasNext = false;
            }
        }
    };
}

There you have the following lines:

action.accept(e);
hasNext = false;

As you throw a RuntimeException inside your lambda (called by using accept(e)), the variable hasNext is never set to false, because the line is not reached.

Upvotes: 1

Related Questions