As @rolfl @rolfl pointed out, think from the perspective of typical use cases to catch this kind of problems, and refactor to avoid repeated calls to hasNext().
As @rolfl @rolfl pointed out, in your next method you don't need to handle the case when there is no next element. Let the underlying iterator take care of it. The user is not supposed to call next anyway without checking hasNext first.
As @rolfl pointed out, think from the perspective of typical use cases to catch this kind of problems, and refactor to avoid repeated calls to hasNext().
As @rolfl pointed out, in your next method you don't need to handle the case when there is no next element. Let the underlying iterator take care of it. The user is not supposed to call next anyway without checking hasNext first.
As @rolfl pointed out, think from the perspective of typical use cases to catch this kind of problems, and refactor to avoid repeated calls to hasNext().
As @rolfl pointed out, in your next method you don't need to handle the case when there is no next element. Let the underlying iterator take care of it. The user is not supposed to call next anyway without checking hasNext first.
Here's an alternative implementation:
Suggested implementation
Here's an alternative implementation:
Suggested implementation
Limiting to Integer type is really unnecessary. It's easy enough to generalize, you can simply change the signature to public class IteratorOfIterator<E> implements Iterator<E> and replace all occurrences of Integer with E, that's it.
The constructor is not ergonomic. You can see this in the unit test, you have to bend over backwards to create a test case: put iterators in a list and then get iterator from that. How about simply a varargs of Iterators:
public IteratorOfIterator(Iterator<E>... iterators) {
// ...
}
As @rolfl pointed out, think from the perspective of typical use cases to catch this kind of problems, and refactor to avoid repeated calls to hasNext().
Your implementation allows null iterators. I think null iterators indicate messy code. If you are working with legacy projects you might be forced to handle these. If that's the case, I think it's better to filter out the null values in the constructor.
As @rolfl pointed out, in your next method you don't need to handle the case when there is no next element. Let the underlying iterator take care of it. The user is not supposed to call next anyway without checking hasNext first.
Here's an alternative implementation:
public class ChainIterator<E> implements Iterator<E> {
private final Iterator<E>[] iterators;
private int index = 0;
public ChainIterator(Iterator<E>... iterators) {
this.iterators = iterators;
}
@Override
public boolean hasNext() {
while (true) {
if (iterators[index].hasNext()) {
return true;
}
if (index == iterators.length - 1) {
return false;
}
++index;
}
}
@Override
public E next() {
return iterators[index].next();
}
@Override
public void remove() {
throw new UnsupportedOperationException();
}
}
public class ChainIteratorTest {
@Test
public void testSingle() {
List<Integer> list1 = Arrays.asList(1, 2, 3);
ChainIterator<Integer> iter2x = new ChainIterator<>(list1.iterator());
for (Integer item : list1) {
assertTrue(iter2x.hasNext());
assertEquals(item, iter2x.next());
}
assertFalse(iter2x.hasNext());
}
@Test
public void testConcatWithManyEmpty() {
List<Integer> list1 = Arrays.asList(1, 2, 3);
List<Integer> list2 = Arrays.asList(4, 5);
ChainIterator<Integer> iter2x = new ChainIterator<>(
Collections.<Integer>emptyList().iterator(),
list1.iterator(),
Collections.<Integer>emptyList().iterator(),
Collections.<Integer>emptyList().iterator(),
list2.iterator());
for (Integer item : list1) {
assertTrue(iter2x.hasNext());
assertEquals(item, iter2x.next());
}
for (Integer item : list2) {
assertTrue(iter2x.hasNext());
assertEquals(item, iter2x.next());
}
assertFalse(iter2x.hasNext());
}
}