Iterating over a range of integer ranges in Java

Question

My use case is this:

RangeDemo.java:

package net.coderodde.util;

public class RangeDemo {

    public static void main(String[] args) {
        for (Integer i : new Ranges(new Range(-9, 0),
                                    new Range(3, 5),
                                    new Range(15, 10),
                                    new Range(11, 11),
                                    new Range(16, 17))) {
            System.out.print(i + " ");
        }
    }
}

... which gives me:

-9 -8 -7 -6 -5 -4 -3 -2 -1 3 4 10 11 12 13 14 16

My implementation is this:

Range.java:

package net.coderodde.util;

/**
 * This class implements a range of integers.
 * 
 * @author Rodion "rodde" EfremoB.
 * @version 1.6 (Jun 10, 2019)
 */
public final class Range {

    private final int fromIndex;
    private final int toIndex;

    public Range(final int fromIndex, final int toIndex) {
        this.fromIndex = Math.min(fromIndex, toIndex);
        this.toIndex   = Math.max(fromIndex, toIndex);
    }

    int get(int index) {
        return fromIndex + index;
    }

    int size() {
        return toIndex - fromIndex;
    }
}

Ranges.java:

package net.coderodde.util;

import java.util.Iterator;
import java.util.NoSuchElementException;
import java.util.Objects;
import java.util.function.Consumer;

/**
 * This class implements an integer iterator over a range of ranges.
 * 
 * @author rodde
 */
public final class Ranges implements Iterable<Integer> {

    /**
     * This static inner class implements the actual iterator.
     */
    private static final class RangeIterator implements Iterator<Integer> {

        /**
         * Holds the actual ranges.
         */
        private final Range[] ranges;

        /**
         * The index of the current range.
         */
        private int currentRangeIndex;

        /**
         * The current index within the current range.
         */
        private int currentElementIndex;

        /**
         * The total number of integers to iterate.
         */
        private final int globalSize;

        /**
         * The global number of integers already iterated.
         */
        private int globalIndex;

        private RangeIterator(Range[] ranges) {
            this.ranges = ranges;
            this.globalSize = countGlobalSize();

        }

        @Override
        public boolean hasNext() {
            return globalIndex < globalSize;
        }

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

            while (currentElementIndex == ranges[currentRangeIndex].size()) {
                currentRangeIndex++;
                currentElementIndex = 0;
            }

            globalIndex++;
            return ranges[currentRangeIndex].get(currentElementIndex++);
        }

        private int countGlobalSize() {
            int globalSize = 0;

            for (Range r : ranges) {
                globalSize += r.size();
            }

            return globalSize;
        }
    }

    private final Range[] ranges;

    public Ranges(Range... ranges) {
        this.ranges = Objects.requireNonNull(ranges);
    }

    @Override
    public Iterator<Integer> iterator() {
        return new RangeIterator(ranges);
    }

    @Override
    public void forEach(Consumer<? super Integer> action) {
        RangeIterator rangeIterator = new RangeIterator(this.ranges);
        rangeIterator.forEachRemaining(action);
    }
}

Answer 1

I think RangeCollection would be a better name than Ranges.

To my mind, the RangeIterator is doing too much. It's iterating over a collection of ranges and over each range in the collection. To me it would make more sense for the Range class to have its own iterator and the RangeCollection its own iterator

Answer 2

This is fairly minor, but I don't think countGlobalSizes is ideal. Its use in the constructor requires that the assignment of ranges happens first; which may cause breakage if you refactor later. It also barely has any reliance on individual instances. I'd make it static and accept the ranges directly as an argument:

private static int sumRangeLengths(Range[] ranges) {
    int totalSize = 0;

    for (Range r : ranges) {
        totalSize += r.size();
    }

    return totalSize;
}

I'm also not a fan of the word "global" being used everywhere. The variables aren't really "global" in most senses. They're private members of instances; which is a pretty constrained scope. They're global to the instance; but every member is, so that's redundant.

---

You may also want to add a step to your range. It's fairly trivial to implement, and is a fairly common aspect of most range implementations.