Pythonish integer range in Java - follow-up

Question

I have refactored the Range; now it looks like this:

Range.java:

package net.coderodde.util;

import java.util.AbstractCollection;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Iterator;
import java.util.List;
import java.util.NoSuchElementException;

/**
 * This class implements an iterator over an arithmetic progression of integers.
 * 
 * @author Rodion "rodde" Efremov
 * @version 1.6 (Dec 1, 2015)
 */
public class Range extends AbstractCollection<Integer>
implements Iterable<Integer> {

    private final int start;
    private final int step;
    private final int stop;

    public static Range range(int start, int stop, int step) {
        return new Range(start, stop, step);
    }

    public static Range range(int start, int stop) {
        return new Range(start, stop, 1);
    }

    public static Range range(int end) {
        return new Range(0, end, 1);
    }

    private Range(int start, int stop, int step) {
        if (step == 0) {
            throw new IllegalArgumentException(
                    "Step argument must not be zero.");
        }

        this.start = start;
        this.step  = step;
        this.stop  = stop;
    }

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

    @Override
    public int size() {
        return Math.max(0, step > 0 ? (stop + step - 1 - start) / step
                                    : (stop + step + 1 - start) / step);
    }

    @Override
    public boolean isEmpty() {
        return size() == 0;
    }

    @Override
    public boolean contains(Object o) {
        try {
            int n = (int)o;

            boolean inBounds = step > 0 ? (start <= n) && (n < stop)
                                        : (start >= n) && (n > stop);

            return inBounds && (n - start) % step == 0;
        } catch (ClassCastException oIsNotAnInt) {
            return false;
        }
    }

    public boolean remove(Object o) {
        throw new UnsupportedOperationException(
                "Removing from an integer Range is not supported.");
    }

    // Throw since if we inherit from AbstractCollection, it will not throw on 
    // empty collection as it will not call method 'add' (which throws).
    @Override
    public boolean addAll(Collection<? extends Integer> c) {
        throw new UnsupportedOperationException(
                "Bulk adding to integer Range is not supported.");
    }

    // Same reasoning as above with 'addAll'.
    @Override
    public boolean removeAll(Collection<?> c) {
        throw new UnsupportedOperationException(
                "Bulk removing from integer Range is not supported.");
    }

    private final class RangeIterator implements Iterator<Integer> {
        private int value = start;

        @Override
        public boolean hasNext() {
            return step > 0 ? value < stop : value > stop;  
        }

        @Override
        public Integer next() {
            if (!hasNext()) {
                throw new NoSuchElementException(
                        "The range iterator exceeded.");
            }

            try {
                return value;
            } finally {
                value += step;
            }
        }
    }

    public static void main(String[] args) {
        Range r = range(100, 9, -3);
        List<Integer> list = new ArrayList<>(r);
        int j = 0;

        for (int i : r) {
            System.out.printf("%3d : %3d\n", i, list.get(j++));
        }
    }
}

RangeTest.java:

package net.coderodde.util;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Set;
import org.junit.Test;
import static org.junit.Assert.*;
import static net.coderodde.util.Range.*;

public class RangeTest {

    @Test
    public void testRange_3args() {
        /////////////////////////////////////////////////////////
        List<Integer> list = new ArrayList<>(range(37, -11, -5));
        /////////////////////////////////////////////////////////

        assertEquals(10, list.size());

        for (int i = 37, j = 0; i > -11; i -= 5, ++j) {
            assertEquals(Integer.valueOf(i), list.get(j));
        }

        Range r = range(10, 50, 7);

        assertTrue(r.contains(10));
        assertTrue(r.contains(17));
        assertTrue(r.contains(24));
        assertTrue(r.contains(31));
        assertTrue(r.contains(38));
        assertTrue(r.contains(45));

        assertEquals(6, r.size());

        Iterator<Integer> iterator = r.iterator();

        for (int i = 10; i < 50; i += 7) {
            assertTrue(iterator.hasNext());
            assertEquals(Integer.valueOf(i), iterator.next());
        }

        assertFalse(iterator.hasNext());

        for (int i = -100; i < 10; ++i) {
            assertFalse(r.contains(i));
        }

        for (int i = 50; i < 100; ++i) {
            assertFalse(r.contains(i));
        }

        ////

        for (int step = -1; step > -10; --step) {
            r = range(5, 50, step);

            assertEquals(0, r.size());
            assertFalse(r.iterator().hasNext());
        }

        for (int step = 1; step < 10; ++step) {
            r = range(50, 5, step);

            assertEquals(0, r.size());
            assertFalse(r.iterator().hasNext());
        }
    }

    @Test
    public void testRange_int_int() {
        Range r = range(10, 17);

        for (int i = 10; i < 17; ++i) {
            assertTrue(r.contains(i));
        }

        assertEquals(7, r.size());
        Iterator<Integer> iterator = r.iterator();

        for (int i = 10; i < 17; ++i) {
            assertTrue(iterator.hasNext());
            assertEquals(Integer.valueOf(i), iterator.next());
        }

        assertFalse(iterator.hasNext());

        for (int i = -100; i < 10; ++i) {
            assertFalse(r.contains(i));
        }

        for (int i = 17; i < 100; ++i) {
            assertFalse(r.contains(i));
        }
    }

    @Test
    public void testRange_int() {
        Range r = range(5);

        for (int i = 0; i < 5; ++i) {
            assertTrue(r.contains(i));
        }

        assertEquals(5, r.size());

        Iterator<Integer> iterator = r.iterator();

        for (int i = 0; i < 5; ++i) {
            assertTrue(iterator.hasNext());
            assertEquals(Integer.valueOf(i), iterator.next());
        }

        assertFalse(iterator.hasNext());
    }

    @Test
    public void testIterator() {
        Range r = range(50, 1, -3);

        assertEquals(17, r.size());
        Iterator<Integer> iterator = r.iterator();

        assertEquals(Integer.valueOf(50), iterator.next());
        assertEquals(Integer.valueOf(47), iterator.next());
        assertEquals(Integer.valueOf(44), iterator.next());
        assertEquals(Integer.valueOf(41), iterator.next());
        assertEquals(Integer.valueOf(38), iterator.next());
        assertEquals(Integer.valueOf(35), iterator.next());
        assertEquals(Integer.valueOf(32), iterator.next());
        assertEquals(Integer.valueOf(29), iterator.next());
        assertEquals(Integer.valueOf(26), iterator.next());
        assertEquals(Integer.valueOf(23), iterator.next());
        assertEquals(Integer.valueOf(20), iterator.next());
        assertEquals(Integer.valueOf(17), iterator.next());
        assertEquals(Integer.valueOf(14), iterator.next());
        assertEquals(Integer.valueOf(11), iterator.next());
        assertEquals(Integer.valueOf(8),  iterator.next());
        assertEquals(Integer.valueOf(5),  iterator.next());
        assertEquals(Integer.valueOf(2),  iterator.next());

        assertFalse(iterator.hasNext());
    }

    @Test
    public void testSize() {
        Range r = range(10, 2, -2);
        assertEquals(4, r.size());

        r = range(2, 10, 2);
        assertEquals(4, r.size());

        r = range(10, 2, 2);
        assertEquals(0, r.size());

        r = range(2, 10, -2);
        assertEquals(0, r.size());
    }

    @Test
    public void testIsEmpty() {
        Range r = range(0);
        assertTrue(r.isEmpty());

        r = range(20, 33, 3);
        assertFalse(r.isEmpty());

        r = range(20, 33, -3);
        assertTrue(r.isEmpty());

        r = range(33, 20, -3);
        assertFalse(r.isEmpty());

        r = range(33, 20, 3);
        assertTrue(r.isEmpty());
    }

    @Test
    public void testContains() {
        Range r = range(10, 0, 2);

        for (int i = 0; i <= 10; i += 2) {
            assertFalse(r.contains(i));
        }

        r = range(10, 1, -2);

        for (int i = 10; i > 1; i -= 2) {
            assertTrue(r.contains(i));
        }

        r = range(10, 0, -2);

        for (int i = 10; i > 0; i -= 2) {
            assertTrue(r.contains(i));
        }

        r = range(10, -1, -2);

        for (int i = 10; i >= 0; i -= 2) {
            assertTrue(r.contains(i));
        }

        ////

        r = range(20, 50, 4);

        for (int i = 20; i < 50; i += 4) {
            assertTrue(r.contains(i));
        }
    }

    @Test
    public void testToArray_0args() {
        Range r = range(-10, 10, 2);
        Object[] arr = r.toArray();

        assertEquals(10, arr.length);

        for (int i = 0, j = -10; i < arr.length; ++i, j += 2) {
            assertEquals(j, arr[i]);
        }

        r = range(10, -10, -2);
        arr = r.toArray();

        assertEquals(10, arr.length);

        for (int i = 0, j = 10; i < arr.length; ++i, j -= 2) {
            assertEquals(j, arr[i]);
        }
    }

    @Test
    public void testToArray_GenericType() {
        Range r = range(37, 21, -1);

        Integer[] input = new Integer[4];
        Integer[] array = r.toArray(input);

        assertFalse(input == array);
        assertEquals(r.size(), array.length);

        input = new Integer[50];

        for (int i = 0; i < input.length; ++i) {
            input[i] = Integer.valueOf(-1);
        }

        array = r.toArray(input);

        assertTrue(array == input);
        assertNull(input[r.size()]);

        for (int i = r.size() + 1; i < input.length; ++i) {
            assertEquals(Integer.valueOf(-1), input[i]);
        }
    }

    public void testContainsAll() {
        Range r = range(20, 3, -3);
        Set<Integer> set = new HashSet<>();
        set.add(20);
        set.add(14);
        set.add(11);
        assertTrue(r.containsAll(set));

        set.add(19);
        assertFalse(r.containsAll(set));
    }

    @Test(expected = UnsupportedOperationException.class)
    public void testAdd() {
        range(10).add(1);
    }

    @Test(expected = UnsupportedOperationException.class)
    public void testRemove() {
        range(10).remove(1);
    }

    @Test(expected = UnsupportedOperationException.class)
    public void testAddAll() {
        range(2, 12).addAll(new HashSet<>());
    }

    @Test(expected = UnsupportedOperationException.class)
    public void testRemoveAll() {
        range(2, 12).removeAll(new HashSet<>());
    }

    @Test(expected = UnsupportedOperationException.class)
    public void testRetainAll() {
        range(2, 12).retainAll(Arrays.asList(1));
    }

    @Test(expected = UnsupportedOperationException.class)
    public void testClear() {
        range(5).clear();
    }
}

So the usage is still the same:

for (int i : range(10, -5, -3)) {
    ...
}

or:

List<Integer> list = new ArrayList<>(range(-5, 10, 3));

I did not however done anything related to IntStream. Am I improving this compared to the first iteration?

Antot · Answer 1 · 2015-12-02 20:01:39Z

Builder Method Names

It's probably a matter of taste, but Range.range(1, 10); and the other two overloaded calls looks a bit redundant. It would look better as Range.of(1, 10);, wouldn't it?

Constants

It's very nice that the three int fields are final, but size() method performs calculations on each invocation of the method (plus if isEmpty() is called!), to produce the same result repetitively.

There should be a final int size field, calculated in the constructor (I wouldn't recommend to use lazy patterns here).

contains(Object)

What will happen if it was called with null arg? A NullPointerException. Since the ClassCastException is caught, I'd vote to add the NullPointer there also.

However, there is an alternative for the try-catch block in this case:

public boolean contains(int n) {
  boolean inBounds = step > 0 ? (start <= n) && (n < stop)
                              : (start >= n) && (n > stop);
  return inBounds && (n - start) % step == 0;
}

@Override
public boolean contains(Object o) {
    return false;
}

Logic

According to the tests and the implementation, ranges like (1, 10, -1), (10, 1, 1) are considered as empty. This choice looks a bit weird, because these ranges have valid bounds, but the value of the step field is inconsistent with these bounds, making them senseless. I think that the constructor arguments should be validated, with an IllegalArgumentException thrown for such cases.

And what about ranges like (1, 10, 20), (10, 1, -20) ? Are they also empty?

If the current implementation does not correspond to a strict requirement, I'd suggest to define an empty range as a range where the start and the stop values are equal.

P.S. please remove the main(args) method from Range class. There is already RangeTest for that.

asked	8 months ago
viewed	89 times
active	6 months ago

current community

your communities

more stack exchange communities

Pythonish integer range in Java - follow-up

1 Answer 1

Builder Method Names

Constants

contains(Object)

Logic

Your Answer

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

Linked

Hot Network Questions

current community

your communities

more stack exchange communities

Pythonish integer range in Java - follow-up

1 Answer 1

Builder Method Names

Constants

contains(Object)

Logic

Your Answer

Sign up or log in

Post as a guest

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

Linked

Related

Hot Network Questions