This Common API Technique is Actually an Anti-Pattern


I admit, we’ve been lured into using this technique as well. It’s just so convenient, as it allows for avoiding a seemingly unnecessary cast. It’s the following technique here:

interface SomeWrapper {
  <T> T get();
}

Now you can type safely assign anything from the wrapper to any type:

SomeWrapper wrapper = ...

// Obviously
Object a = wrapper.get();

// Well...
Number b = wrapper.get();

// Risky
String[][] c = wrapper.get();

// Unprobable
javax.persistence.SqlResultSetMapping d = 
    wrapper.get();

This is actually the API you can use when you’re using jOOR, our reflection library that we’ve written and open sourced to improve our integration tests. With jOOR, you can write things like:

Employee[] employees = on(department)
    .call("getEmployees").get();
 
for (Employee employee : employees) {
    Street street = on(employee)
        .call("getAddress")
        .call("getStreet")
        .get();
    System.out.println(street);
}

The API is rather simple. The on() method wraps an Object or a Class. The call() methods then call a method on that object using reflection (but without requiring exact signatures, without requiring the method to be public, and without throwing any checked exceptions). And without the need for casting, you can then call get() to assign the result to any arbitrary reference type.

This is probably OK with a reflection library like jOOR, because the whole library is not really type safe. It can’t be, because it’s reflection.

But the “dirty” feeling remains. The feeling of giving the call-site a promise with respect to the resulting type, a promise that cannot be kept, and that will result in ClassCastException – a thing of the past that junior developers who have started after Java 5 and generics hardly know.

But the JDK libraries also do that…

Yes, they do. But very seldomly, and only if the generic type parameter is really irrelevant. For instance, when getting a Collection.emptyList(), whose implementation looks like this:

@SuppressWarnings("unchecked")
public static final <T> List<T> emptyList() {
    return (List<T>) EMPTY_LIST;
}

It’s true that the EMPTY_LIST is cast unsafely from List to List<T>, but from a semantic perspective, this is a safe cast. You cannot modify this List reference, and because it’s empty, there is no method in List<T> that will ever give you an instance of T or T[] that does not correspond to your target type. So, all of these are valid:

// perfectly fine
List<?> a = emptyList();

// yep
List<Object> b = emptyList();

// alright
List<Number> c = emptyList();

// no problem
List<String[][]> d = emptyList();

// if you must
List<javax.persistence.SqlResultSetMapping> e 
    = emptyList();

So, as always (or mostly), the JDK library designers have taken great care not to make any false promises about the generic type that you might get. This means that you will often get an Object type where you know that another type would be more suitable.

But even if YOU know this, the compiler won’t. Erasure comes at a price and the price is paid when your wrapper or collection is empty. There is no way of knowing the contained type of such an expression, so don’t pretend you do. In other words:

Do not use the just-to-avoid-casting generic method anti pattern

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s