2
\$\begingroup\$

GlueArrays is used for iterating over an array of arrays, and it looks like this:

Main.java

import java.util.Iterator;
import java.util.NoSuchElementException;

public class Main {

    public static class GlueArrays<T> implements Iterable<T> {

        private final T[][] arrays;

        public GlueArrays(T[]... arrays) {
            this.arrays = arrays;
        }

        @Override
        public Iterator<T> iterator() {
            return new GlueArrayIterator(arrays);
        }

        private static class GlueArrayIterator<T> implements Iterator<T> {

            private final T[][] arrays;
            private int arrayIndex = 0;
            private int localIndex = 0;

            GlueArrayIterator(T[][] arrays) {
                this.arrays = arrays;
            }

            @Override
            public boolean hasNext() {
                if (arrayIndex == arrays.length) {
                    return false;
                }

                return arrayIndex < arrays.length ||
                       localIndex < arrays[arrayIndex].length;
            }

            @Override
            public T next() {
                if (!hasNext()) {
                    throw new NoSuchElementException("Nothing to iterate.");
                }

                T returnValue = arrays[arrayIndex][localIndex];

                if (localIndex < arrays[arrayIndex].length) {
                    localIndex++;

                    if (localIndex == arrays[arrayIndex].length) {
                        localIndex = 0;
                        arrayIndex++;
                    }
                }

                return returnValue;
            }
        }
    }
    public static void main(String[] args) {
        Number[] arr1 = {1, 2, 3};
        Number[] arr2 = {4f, 5f, 6f};
        Number[] arr3 = {7.0, 8.0, 9.0};

        for (Number number : new GlueArrays<>(arr1, arr2, arr3)) {
            System.out.println(number);
        }
    }
}

As always, any critique is welcome.

\$\endgroup\$
6
\$\begingroup\$

First off, this is buggy. If one of the input arrays is empty, you'll get an ArrayIndexOutOfBoundsException - and even if you correct that error, the iterator will stop if it encounters an empty array.

Consider using proper tests with multiple scenarios, especially with edge cases, in order to catch errors like that.


Then I get two warnings from Eclipse (other compilers and their settings may differ, but I believe these are "common" warnings, that normally would be raised). You should never have warnings. Code should preferably be adjusted or rewritten to avoid them, or at the very least (if you have a good reason to keep the warning) annotated with @SuppressWarnings and commented on with those reasons.

1) Due to the use of generics together with variable arguments the GlueArrays constructor should have the annotation @SafeVarArgs.

2) The instantiation of the iterator in iterate() is lacking its type parameters. It should be

return new GlueArrayIterator<T>(arrays);

or, if you are at least using Java 1.7, just

return new GlueArrayIterator<>(arrays);
\$\endgroup\$
  • \$\begingroup\$ Thanks for reporting the bugs! \$\endgroup\$ – coderodde Mar 12 at 13:20
-1
\$\begingroup\$

Do you really need an iterator? You could do the same with three lines of Streams and flat mapping:

Arrays.stream(twoDimensionalArray)
    .flatMap(array -> Arrays.stream(array))
    .forEach(element -> System.err.println(element));
\$\endgroup\$

Your Answer

By clicking "Post Your Answer", you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.