HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Iterator of iterator

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
iteratorstackoverflowprogramming

Problem

Given an iterator of iterators, return all of the elements from the iterators in order. Looking for code review, optimizations, and best practices. Verifying complexity to be \$O(n)\$, where \$n\$ is the total number of elements from all iterators.

public class IteratorOfIterator implements Iterator{

    private final Iterator> iteratorOfIterator;
    private Iterator currentIterator;

    public IteratorOfIterator(Iterator> iterator)  {
        this.iteratorOfIterator = iterator;
    }

    @Override
    public boolean hasNext() {
        while (currentIterator == null || !currentIterator.hasNext()) {
            if (!iteratorOfIterator.hasNext()) return false;
            currentIterator = iteratorOfIterator.next();
        }
        return true;
    }

    @Override
    public Integer next() {
        if (!hasNext()) {
            throw new NoSuchElementException("the stuff cannot be null.");
        }
        return currentIterator.next();
    }

    @Override
    public void remove() {
        throw new UnsupportedOperationException("The remove operation is not supported.");
    }
}

public class IteratorOfIteratorTest {

    @Test
    public void testIterator() {
        List list1 = new ArrayList<>(Arrays.asList(1, 2, 3, 4));
        List list2 = new ArrayList<>(Arrays.asList(5, 6, 7, 8));
        List> combined = new ArrayList<>(Arrays.asList(list1.iterator(), null, list2.iterator()));

        IteratorOfIterator ioi = new IteratorOfIterator(combined.iterator());
        int[] expected = {1, 2, 3, 4, 5, 6, 7, 8};
        int[] actual = new int[8];
        int i = 0;
        while (ioi.hasNext()) {
             actual[i] = ioi.next();
             i++;
        }
        assertArrayEquals(expected, actual);
    }
}

Solution

I don't see any reason to limit this to Integers. The solution can be trivially genericized.

IteratorOfIterator is a clumsy name. By analogy with Python's itertools.chain(), I suggest calling this class ChainIterator. You can rename the iteratorOfIterator private variable accordingly, too.

I suggest adding an alternate constructor that takes an Iterable>, for convenience.

The hasNext() method seems fine. As for next(), the message in the NoSuchElementException that you throw seems a bit weird; I would just omit it.

With not much extra effort, you can add support for remove().

public class ChainIterator implements Iterator {

    private final Iterator> chain;
    private Iterator currentIterator;
    private Iterator lastIterator;

    public ChainIterator(Iterable> iterable)  {
        this(iterable.iterator());
    }

    public ChainIterator(Iterator> iterator)  {
        this.chain = iterator;
    }

    @Override
    public boolean hasNext() {
        while (currentIterator == null || !currentIterator.hasNext()) {
            if (!chain.hasNext()) return false;
            currentIterator = chain.next();
        }
        return true;
    }

    @Override
    public T next() {
        if (!this.hasNext()) {
            this.lastIterator = null;         // to disallow remove()
            throw new NoSuchElementException();
        }
        this.lastIterator = currentIterator;  // to support remove()
        return currentIterator.next();
    }

    @Override
    public void remove() {
        if (this.lastIterator == null) {
            throw new IllegalStateException();
        }
        this.lastIterator.remove();
    }
}

Code Snippets

public class ChainIterator<T> implements Iterator<T> {

    private final Iterator<Iterator<T>> chain;
    private Iterator<T> currentIterator;
    private Iterator<T> lastIterator;

    public ChainIterator(Iterable<Iterator<T>> iterable)  {
        this(iterable.iterator());
    }

    public ChainIterator(Iterator<Iterator<T>> iterator)  {
        this.chain = iterator;
    }

    @Override
    public boolean hasNext() {
        while (currentIterator == null || !currentIterator.hasNext()) {
            if (!chain.hasNext()) return false;
            currentIterator = chain.next();
        }
        return true;
    }

    @Override
    public T next() {
        if (!this.hasNext()) {
            this.lastIterator = null;         // to disallow remove()
            throw new NoSuchElementException();
        }
        this.lastIterator = currentIterator;  // to support remove()
        return currentIterator.next();
    }

    @Override
    public void remove() {
        if (this.lastIterator == null) {
            throw new IllegalStateException();
        }
        this.lastIterator.remove();
    }
}

Context

StackExchange Code Review Q#56575, answer score: 6

Revisions (0)

No revisions yet.