Covariant return type best practices

Question

I vaguely remember learning in university that the return type of a method should always be as narrow as possible, but my search for any references online came up empty and SonarQube calls it a code smell. E.g. in the following example (note that TreeSet and Set are just examples)

public interface NumberGetter {
    Number get();

    Set<Number> getAll();
}

public class IntegerGetter implements NumberGetter {

    @Override
    public Integer get() {
        return 1;
    }

    @Override
    public TreeSet<Number> getAll() {
        return new TreeSet<>(Collections.singleton(1));
    }
}

SonarQube tells me

Declarations should use Java collection interfaces such as "List" rather than specific implementation classes such as "LinkedList". The purpose of the Java Collections API is to provide a well defined hierarchy of interfaces in order to hide implementation details. (squid:S1319)

I see the point about hiding implementation details, since I cannot easily change the return type of IntegerGetter::getAll() without breaking backwards-compatibility. But by doing this, I also provide the consumers with potentially valuable information, i.e. they could change their algorithm to be more appropriate for using a TreeSet. And if they don't care about this property, they can still just use IntegerGetter (however they obtained it), like this:

Set<Number> allNumbers = integerGetter.getAll();

So I have the following questions:

• Is it appropriate for IntegerGetter::get() to return a narrower type?
• Is it appropriate for IntegerGetter::getAll() to return a narrower type?
• Are there any best practices regarding this topic or is the answer just "it depends"?

(N.B. SonarQube doesn't complain if I replace TreeSet with e.g. SortedSet. Is this code smell only about not using a Collection API interface? What if there is only a concrete class for my particular problem?)

Answer 1

This cannot have a single answer, as it depends on the usecase.
And by that I mean that it depends on the degree of flexibility you want your implementation to have, plus the degree of flexibility you want to give the consumers of your API.

I'll give you a more general answer.

Do you want your consumer to loop-only? Return a Collection<T>.
Do you want your consumer to be able to access by index? Return a List<T>
Do you want your consumer to know and be able to efficiently verify if an element is present? Return a Set<T>
And so on.

The same approach is valid for input parameters. What's the point of accepting a List<T>, or even concrete classes such as ArrayList<T> or LinkedList<T>, if you only loop it? You are just giving your code less flexibility for future modifications.

What you're doing here with the IntegerGetter's inherited methods' return types is called type specialization. It's okay as long as you keep exposing interfaces to the outer world.

My rule of thumb is being as generic as possibile when dealing with the outer world, and being as specific as possible when implementing critical (core) parts of my application, to restrict the possible use-cases, protect myself from abusing what I just coded, and for documentation purposes.

What you shouldn't absolutely be doing is using instanceof or class comparison to discover the actual type and take different routes. That's ruining a codebase.

Answer 2

The return type must strike a balance between the needs of the caller and the needs of the implementation: the more you tell a caller about the return type, the harder it is to change that type later.

Which is more important will depend on the specific circumstances. Do you ever see this type changing? How valuable is knowing the type for the caller?

In the case of IntegerGetter.get(), it would be very surprising if the return type ever changes, so telling the caller does no harm.

In the case of IntegerGetter.getAll(), it depends on what the caller uses the method for:

• If he merely wants to iterate, an Iterable would be the right choice.
• If we need more methods such as size, Collection might.
• If he relies on the numbers being unique, a Set.
• If he additionally relies on the numbers being sorted, a SortedSet.
• And if it actually needs to be the red black tree from the JDK so it can manipulate its internal state directly for an ugly hack, a TreeSet might be the right choice.

Answer 3

I try - as a rule of thumb - to use on method signature the most general type (class or interface does'n matter) that supports the API I need: more general the type, more minimal the API. So, if I need a parameter representing a family of object of the same type, I start using Collection; if in the specific scenario is the idea of ordering important, I use List, avoiding to publish any info about the specific List implementation I use internally: my idea is to keep the ability to change implementation (may be for performance optimization, to support a different data structore, and so on) without break clients.

As you stated, publishing information like I use a TreSet can leave place for client-side optimization - but my idea is that it depends: case by case you can evaluate if the specific scenario requires to relax the general rule to expose the more general interface you can.

So, coming to your questions:

1. yes, it is appropriate in the IntegerGetter implementation of NumberGetter interface to return a narrower type: Java allows you to do and you are not breaking my more generic is more beautiful rule: the NumberGetter interface exposes the more general interface using Number as return type but in specific implementations we can use a narrower return type to guide the method implementation: clients referring to the more abstract interface are not affected by this choice and client referring to the specific subclass can try advantage from using the more concrete interface
2. the same as the previous point, but I think it's less useful than in the previous case for clients: may be a client can find useful to refer to Integer than to Number (if I use explicitly a NumberGetter, may be I think in terms of Integers, not in term of Numbers), but referring to TreeSet rather than to Set is useful only if you need the API exposed by the subclass and not by the interface...
3. see initial dissertation

It's a quasi-philosophic question - and so is my answer: I hope it can be useful to you!