Pythonish integer range in Java - follow-up

Question

(See the previous iteration.)

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?

Answer 1

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.

Answer 2

The RangeTest is incomplete.

• It should have tests for ranges near Integer.MIN_VALUE and Integer.MAX_VALUE.
• How does range(MAX - 1, MAX, 3) behave?
• Does range(MIN, MAX) contain 0? Does it contain MIN? Does it contain MAX?
• Does range(MIN, MAX, 5) contain 0, 1, 2, 3, 4? Does it contain MIN? Does it contain MAX?
• Can the range be endless by accident?