Is it safe to silently catch ClassCastException when searching for a specific value?
-
10-10-2019 - |
Question
Suppose I am implementing a sorted collection (simple example - a Set
based on a sorted array.) Consider this (incomplete) implementation:
import java.util.*;
public class SortedArraySet<E> extends AbstractSet<E> {
@SuppressWarnings("unchecked")
public SortedArraySet(Collection<E> source, Comparator<E> comparator) {
this.comparator = (Comparator<Object>) comparator;
this.array = source.toArray();
Arrays.sort(this.array, this.comparator);
}
@Override
public boolean contains(Object key) {
return Arrays.binarySearch(array, key, comparator) >= 0;
}
private final Object[] array;
private final Comparator<Object> comparator;
}
Now let's create a set of integers
Set<Integer> s = new SortedArraySet<Integer>(Arrays.asList(1, 2, 3), null);
And test whether it contains some specific values:
System.out.println(s.contains(2));
System.out.println(s.contains(42));
System.out.println(s.contains("42"));
The third line above will throw a ClassCastException
. Not what I want. I would prefer it to return false
(as HashSet
does.)
I can get this behaviour by catching the exception and returning false:
@Override
public boolean contains(Object key) {
try {
return Arrays.binarySearch(array, key, comparator) >= 0;
} catch (ClassCastException e) {
return false;
}
}
Assuming the source
collection is correctly typed, what could go wrong if I do this?
Solution
I don't think there is any issue with this as the Javadoc for Collection.contains
clearly states that throwing a ClassCastException
is optional.
The only issue I see is that if you have a bug somewhere not throwing an exception will prevent you to pinpoint it.
OTHER TIPS
The TreeSet
class does throw a ClassCastException
for incompatible arguments to contains()
(incompatible for the Comparator
used by the set). So there is nothing wrong with throwing that exception. Just make sure you document that this may happen.
It is perfectly legitimate to let a CCE throw from contains(). However, many collection implementations catch that and return false, which I consider to also be perfectly legitimate, and in fact is the more user-friendly behavior.
In equals() you don't have the choice; you have to catch that CCE.
Catching an unchecked exception should always feel dirty, but sometimes it is the right thing to do.