1

Given this generic class and constructor,

class A<E extends Number> {

    A(E number, Comparable<E> comparable) {
        //...
    }
}

it is ensured that a call to the generic constructor will check the arguments' types at compile time:

new A<>(Integer.valueOf(4), Integer.valueOf(4)); // ok
new A<>(Integer.valueOf(4), Long.valueOf(4)); // error: Cannot infer type arguments for A<>

(of course, even explicitly stating the parametrized type instead of letting the compiler infer the type gives the same end result because the relation cannot be satisfied).

However, the raw constructor allows both calls to compile:

new A(Integer.valueOf(4), Integer.valueOf(4));
new A(Integer.valueOf(4), Long.valueOf(4));

with the warning on both invocations:

Type safety: The constructor A(Number, Comparable) belongs to the raw type A. References to generic type A<E> should be parameterized

My question is, should the constructor perform type checks (e.g., instanceof, isAssignableFrom or isInstance) in its body to ensure that the class is not created in an invalid state in case the raw constructor is called?
These type checks are completely redundant if the constructor call is parametrized. Is the compiler warning about type safety a good enough excuse to not do these checks?

Additional details:

  • This class is public API of a library.
  • The usage is documented.
  • Performance is of no concern whatsoever.
  • This sounds like a judgment call to me. – Robert Harvey Jul 22 at 15:18
1

The declaration

class A<E extends Number> {

    A(E number, Comparable<E> comparable) {
        //...
    }
}

is a bit strange, as the intention seems to be that both constructor arguments have the same type, but actually, it allows invocations like new A<>(new AtomicInteger(), x -> 0), where the first argument is a number and the second implements Comparable for that.

So the declaration should rather be

class A<E extends Number & Comparable<? super E>> {
    A(E number1, E number2) {
        //...
    }
}

But it has to be emphasized that this still doesn’t imply that number1 and number2 are instances of the same class, as that’s not an actual requirement. While the assumption of having the same type holds for Integer or Long, there could be a different number type fulfilling the constraints but having specific subclasses. In that case, number1 and number2 could be of different runtime classes, neither of them being the actual E, but the actual E will be a common base class.

This is actually a typical situation for generic code; we don’t know the type arguments specified or inferred for the type parameters and any attempt to perform checks based on runtime types can lead to substantial wrong answers. So the simple answer is that you shouldn’t try to check for the correct generic usage, it’s usually not only expensive, but also error prone.

Still, there’s something we should test. Java’s generic type system doesn’t support ruling out null values, hence, we may have to check for them, especially in situations where we are going to store values and not recognizing null can lead to exceptions at a significantly later time, where it is hard to debug the origin of the null.

When we insert a simple check at the start

class A<E extends Number & Comparable<? super E>> {
    A(E number1, E number2) {
        number1.compareTo(number2);
        //...
    }
}

We’re ruling out null for number1 immediately and also for number2, if the underlying comparable implementation is correct (and we’re not getting very far if we assume incorrect code everywhere). In fact, we even get a solution for the original request for free; if the caller used raw types to insert incompatible objects, we get an exception right at this point. This is different to any attempt to use instanceof et al, we still don’t know the actual E, but we know for sure that the objects have types extending Number, implementing Comparable, and that an actual comparison attempt can succeed.

Note that TreeMap does a similar thing when the first key is entered. Since there is no other key to compare, but incompatible values could stay unnoticed for a long time, it compares the key to itself, to fail-fast. This had to be learned the hard way

0

This question depends on how and to who you expose this class.

Will users use this class as a library "on-the-fly", or in other words won't be aware to such warning when they program against it. Then, I would say that adding such simple run-time checks are appropriate.
Since, an "outsider" user may not be aware to such limitation, and if it's not documented it may lead to insidious/hard to debug errors for such user. So, the run-time check has a significant value in this case.

If this class is used by a small group, and such warnings are checked by its engineers - it can be documented and being adhered/enforced by this group. So, in this case this check value is insignificant.

(Regarding performance, the "hit" of such check can be regarded as insignificant - but it creates unnecessary clutter in the code that can be avoided)

  • I added additional info to my question. I don't understand most of your points. (1) What is to use a library "on-the-fly"? (2) Users that program against any API are aware of the compiler warnings (though they may ignore them). (3) What is an "outsider"? – user1803551 Jul 22 at 23:39
  • 1
    @user1803551: In general terms, what (s)he is saying is that you need more rigor if it's a publicly-available library than if it's just an internal class in a corporate project. – Robert Harvey Jul 23 at 0:01
  • @RobertHarvey (He* :) ) and yes that's my point. – nadir Jul 23 at 10:30
  • @nadir: Yeah, sorry. There's no good way to do gender nowadays. – Robert Harvey Jul 23 at 14:05

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.