Problem
I made a Range class (like an Interval) that takes Integers and implements List<Integer> (actually it implements UnmodList, but whatever). The contains(), indexOf(), and lastIndexOf() methods of java.util.List can be implemented efficiently (using math instead of iteration) in a Range, making the List interface fit Ranges better than it does Lists!
Well, except for contains(Object o). I think there is consensus today that this method signature should be contains(T t) for a List<T>, but they didn’t know that when they added Generics in Java 1.5. Which means you can pass all kinds of ridiculous things to this method. Since the underlying range is being stored as primitive int values, it’s limited to Integer.MIN_VALUE and Integer.MAX_VALUE. Which is fine everywhere except the contains() method.
Specifically, how do I account for (should I account for) wacky implementations of Number or other classes that might have an intValue() or intValueExact() method? Also, I really want to warn/prevent accidental truncation, so that RangeOfInt.of(-1, 2).contains(Long.valueOf(Integer.MAX_VALUE + 1L)) does not return true.
// An efficient method for when the compiler can see that
// it's being passed a primitive int.
public boolean contains(int i) {
return (i >= start) && (i < end);
}
@Override public boolean contains(Object o) {
// Yuck. Why couldn't we just have to deal with an Integer here?
// As it stands, we attempt to prevent overflow/underflow.
if (o instanceof Integer) {
return contains(((Integer) o).intValue());
} else if (o instanceof Long) {
long l = (Long) o;
return (l <= ((long) Integer.MAX_VALUE)) &&
(l >= ((long) Integer.MIN_VALUE)) &&
contains((int) l);
} else if (o instanceof BigInteger) {
try {
// Throws an exception if it's more than 32 bits.
return contains(((BigInteger) o).intValueExact());
} catch (ArithmeticException ignore) {
return false;
}
} else if (o instanceof Number) {
// No way to check this one. Does that mean the above are misleading?
return contains(((Number) o).intValue());
}
return false;
}
My primary question is whether or not detecting Numbers is a good thing or a bad thing. The good error handling of Longs and BigIntegers could make people expect reasonable error checking when sending Numbers too! Float and Double are Numbers, but auto-converting them to an int gives rise to wacky stuff like:
RangeOfInt.of(0, 9).contains(2.5); // true.
I guess I’m talking myself out of supporting Number – you can call .contains(myNumber.intValue()) if you need to.
My second-biggest question is that Josh Bloch’s Item 41 advises against overloading (not overriding) methods with the same number of arguments. Since this one form of the method takes a primitive int, the only way it could be confused with an Object is through auto-boxing, but if it’s boxed to an Integer that case will be handled just fine. Does that make this an OK exception to Bloch’s overloading rule?
You may find it helpful to look at the unit test for this contains() method.
Solution
Similar to @Smac89‘s suggestion, there are two things to consider:
- Whether you want to be lenient in your comparison.
- Whether you want to silently return
falsefor non-integers, or throw anException.
For the first, you should simply rely on what Number.intValue() tells you, regardless of whether “rounding or truncation” (Javadoc) was involved. On a related note, you can refer to what the Java Language Specifications say on narrowing primitive conversions. Document this carefully in the method’s Javadoc too, something along the lines of:
/**
* Uses {@link Number#intValue()} for the conversion.
* @param o the Object to compare
* @return {@code true} if the converted value exists in this range
*/
public boolean contains(Object o) {
return o instanceof Number && contains(((Number) o).intValue());
}
For the second, while List.contains() says that implementations can throw ClassCastException “if the type of the specified element is incompatible”, it is marked as an optional restriction. If you surmise that getting a value 2.5 for this particular method indicates something is really wrong at runtime, you should then throw an Exception:
/**
* @param o the Object to compare
* @return {@code true} if the argument is an {@link Integer} instance
* and its primitive value exists in this range
* @throws ClassCastException for non-{@link Integer} arguments
*/
public boolean contains(Object o) {
if (o instanceof Integer) {
return contains(((Integer) o).intValue());
}
throw new ClassCastException("Not an integer");
}
Let Java help you. Here is how I would rewrite that mess of code for the contains method:
@Override
public boolean contains(Object o) {
try {
Number number = (Number)o;
int value = Integer.valueOf(number.toString()).intValue();
return contains(value);
} catch(NumberFormatException | ClassCastException e) {}
return false;
}
Double
Your question about supporting doubles is good because this makes the class robust, however I think you might be deviating from the purpose of this class.
Your class is called RangeOfInt so keep it that way. If the user decides to use doubles for your class, then they should be aware that whatever result they get is undefined because the class is built for integers not doubles. Think of your class as the range method in python. Now if the user types RangeOfInt.of(0, 9).contains(2.5), since this class was to be used for integers, either throw an exception (which is the right thing to do) or simply let your implementation run it’s course and return whatever it wants. Simply put, the user does something undefined, something undefined should happen in return.
Overloading
The way you have used overload is good because it keeps the purpose of the class evident. By having one “Object” method, that handles everything that is not integer, you make it obvious that the other method with just an integer as a parameter has a special purpose for int only parameters.