Saturday 7 July 2012

Item 24: Eliminate unchecked warnings


When you program with generics, you will see many compiler warnings: unchecked cast warnings, unchecked method invocation warnings, unchecked generic array creation warnings, and unchecked conversion warnings.

Many unchecked warnings are easy to eliminate. For example, suppose you accidentally write this declaration:

Set<Lark> exaltation = new HashSet();

The compiler will gently remind you what you did wrong:

Venery.java:4: warning: [unchecked] unchecked conversion
found : HashSet, required: Set<Lark>
Set<Lark> exaltation = new HashSet();
^

You can then make the indicated correction, causing the warning to disappear:

Set<Lark> exaltation = new HashSet<Lark>();

Some warnings will be much more difficult to eliminate. Eliminate every unchecked warning that you can. If you eliminate all warnings, you are assured that your code is typesafe.

If you can’t eliminate a warning, and you can prove that the code that provoked the warning is typesafe, then (and only then) suppress the warning with an @SuppressWarnings("unchecked") annotation. If you suppress warnings without first proving that the code is typesafe, you are only giving yourself a false sense of security. The code may compile without emitting any warnings, but it can still throw a ClassCastException at runtime.

The SuppressWarnings annotation can be used at any granularity, from an individual local variable declaration to an entire class. Always use the Suppress-Warnings annotation on the smallest scope possible. Typically this will be a variable declaration or a very short method or constructor. Never use Suppress- Warnings on an entire class. Doing so could mask critical warnings.

For example, consider this toArray method, which comes from ArrayList:

public <T> T[] toArray(T[] a) {
if (a.length < size)
return (T[]) Arrays.copyOf(elements, size, a.getClass());
System.arraycopy(elements, 0, a, 0, size);
if (a.length > size)
a[size] = null;
return a;
}

If you compile ArrayList, the method generates this warning:

ArrayList.java:305: warning: [unchecked] unchecked cast
found : Object[], required: T[]
return (T[]) Arrays.copyOf(elements, size, a.getClass());
^

It is illegal to put a SuppressWarnings annotation on the return statement, because it isn’t a declaration [JLS, 9.7]. You might be tempted to put the annotation on the entire method, but don’t. Instead, declare a local variable to hold the return value and annotate its declaration, like so:

// Adding local variable to reduce scope of @SuppressWarnings
public <T> T[] toArray(T[] a) {
if (a.length < size) {
// This cast is correct because the array we're creating
// is of the same type as the one passed in, which is T[].
@SuppressWarnings("unchecked") T[] result =
(T[]) Arrays.copyOf(elements, size, a.getClass());
return result;
}
System.arraycopy(elements, 0, a, 0, size);
if (a.length > size)
a[size] = null;
return a;
}

Every time you use an @SuppressWarnings("unchecked") annotation, add a comment saying why it’s safe to do so. This will help others understand the code, and more importantly, it will decrease the odds that someone will modify the code so as to make the computation unsafe.

In summary, unchecked warnings are important. Don’t ignore them. Every unchecked warning represents the potential for a ClassCastException at runtime. Do your best to eliminate these warnings. If you can’t eliminate an unchecked warning and you can prove that the code that provoked it is typesafe, suppress the warning with an @SuppressWarnings("unchecked") annotation in
the narrowest possible scope. Record the rationale for your decision to suppress the warning in a comment.


Reference: Effective Java 2nd Edition by Joshua Bloch