Java Unchecked Overriding Return Type

Eliezer picture Eliezer · May 14, 2014 · Viewed 8.6k times · Source

I have a project that has the following components:

public abstract class BaseThing {
    public abstract <T extends BaseThing> ThingDoer<T, String> getThingDoer();
}

public class SomeThing extends BaseThing {
    public ThingDoer<SomeThing, String> getThingDoer() {
        return Things.getSomeThingDoer();
    }
}

public class SomeOtherThing extends BaseThing {
    public ThingDoer<SomeOtherThing, String> getThingDoer() {
        return Things.getSomeOtherThingDoer();
    }
}

public class Things {
    public ThingDoer<SomeThing, String> getSomeThingDoer {
        return getThingDoer(SomeThing.class);
    }

    public ThingDoer<SomeOtherThing, String> getSomeOtherThingDoer {
        return getThingDoer(SomeOtherThing.class);
    }

    private <D extends ThingDoer<T, String> D getThingDoer(Class<T> clazz) {
        //get ThingDoer
    }
}

public class ThingDoer<T, V> {
    public void do(T thing) {
        //do thing
    }
}

public class DoThing {
    private BaseThing thing;

    public void doIt() {
        thing.getThingDoer().do(thing);
    }
}

I'm getting a compiler warning in SomeThing.getThingDoer() that says:

Unchecked overriding: return type requires unchecked conversion.

Found ThingDoer<SomeThing, String>, required ThingDoer<T, String>

Everthing compiles fine, and while I didn't get a chance to test out DoThing.doIt() yet, I have no reason to believe that it won't work.

My question is, can this break and is there a better way to do this? I could make DoThing a base class, and have subclasses for both SomeThing and SomeOtherThing but that doesn't seem very elegant.

EDIT: I would like to avoid making BaseThing generic.

Answer

William Price picture William Price · May 14, 2014

Let's look first at your BaseThing class that you don't want to make generic:

public abstract class BaseThing {
    public abstract <T extends BaseThing> ThingDoer<T, String> getThingDoer();
}

This is not a generic class, but it contains a generic method. Frequently, generic methods like this are designed so that the type <T> is bound by the compiler based on some argument to the method. For example: public <T> Class<T> classOf(T object). But in your case, your method takes no arguments. That is also somewhat common, in cases where the implementation of the method returns something "universally" generic (my term) like this method from the Collections utility class: public <T> List<T> emptyList(). This method takes no arguments, but the type <T> will be inferred from the calling context; it works only because the implementation of emptyList() returns an object that is type-safe in all cases. Due to type erasure, the method doesn't ever actually know the type of T when it's called.

Now, back to your classes. When you create these subclasses of BaseThing:

public class SomeThing extends BaseThing {
    public ThingDoer<SomeThing, String> getThingDoer() {
        return Things.getSomeThingDoer();
    }
}

public class SomeOtherThing extends BaseThing {
    public ThingDoer<SomeOtherThing, String> getThingDoer() {
        return Things.getSomeOtherThingDoer();
    }
}

Here, you want to override the abstract method from the base class. Overriding the return type is allowed in Java as long as the return type is still valid in the context of the original method. For instance you can override a method that returns Number with a specific implementation that always returns Integer for that method, because Integer is a Number.

With generics, however, a List<Integer> is not a List<Number>. So while your abstract method is defined to return ThingDoer<T, String> (for some T extends BaseThing), your overloads that return ThingDoer<SomeThing, String> and ThingDoer<SomeOtherThing, String> are not generally compatible with some unknown T even though SomeThing and SomeOtherThing both extend from BaseThing.

The caller (from the abstract API) expects some unknown, unenforceable T that cannot be guaranteed to be satisfied by either of your concrete implementations. In fact, your concrete overloads are no longer generic (they return specific, statically-bound type parameters) and that conflicts with the definition in the abstract class.

EDIT: The "correct" way (no warnings) to define the abstract method should be something like:

public abstract ThingDoer<? extends BaseThing, String> getThingDoer();

This makes it clear to the caller that it's getting a ThingDoer with its first type parameter bound to something that extends BaseThing (so it can use it as if it were a BaseThing) but the caller will not know the specific implementation when accessed by the abstract API.

EDIT #2 - Findings from our discussion in chat...

The OP's original example usage is:

BaseThing thing = /* ... */;
thing.getThingDoer().do(thing);

Notice how the same thing reference is passed back into a method in the object returned from that same thing's getThingDoer() method. The object returned by getThingDoer() needs to be tightly bound to the concrete implementation type of thing (according to the OP). To me, this smells like broken encapsulation.

Instead, I suggest exposing the logical operation as a part of the BaseThing API and encapsulating the delegation to the ThingDoer as an internal implementation detail. The resulting API would look something like:

thing.doTheThing();

And implemented somewhat like:

public class SomeThing extends BaseThing {
    @Override public void doTheThing() {
        Things.getSomeThingDoer().do(this);
    }
}

public class SomeOtherThing extends BaseThing {
    @Override public void doTheThing() {
        Things.getSomeOtherThingDoer().do(this);
    }
}