2

I want to remove the suppression of the unchecked conversion warning. How can I cast o to T with complete surety?

Of note: the ResultSet in this snippet is a proprietary wrapper that is source-agnostic. It's very similar to java.sql.ResultSet, but it is not the same. Also, using Eclipse Mars.1 with Java 8 u45, and both the IDE and javac issue the warning. I realize that since it's wrapped in that if statement, that it's technically not an issue, but I absolutely hate to suppress warnings. And I feel like there's gotta be a completely type-safe way to perform that conversion.

public class ResultSetQuery {
    public static <T> List<T> collectValues(ResultSet rs, String keyName, Class<T> tclass) {

        List<T> result = new LinkedList<>();

        while(rs.next()) {
            Object o = rs.getData(keyName);

            if (tclass.isAssignableFrom(o.getClass())) {

                @SuppressWarnings("unchecked")
                T v = (T)o;

                result.add(v);
            } else {
                result.add(null);
            }
        }

        return result;
    }
}
liltitus27
  • 1,670
  • 5
  • 29
  • 46
  • why do you bother? The test `isAssignableFrom` makes sure that the cast cannot fail, so in theory the compiler could be smart enough to not issue the warning, but anyway it is safe to suppress the warning. – wero Dec 28 '15 at 16:46
  • Is the ResultSet from `java.sql` package? I didn't find the `getData` method in the official documentation. – Philipe Dec 28 '15 at 16:47
  • `while (rs.next())` can throw an `SQLException`, so you need to either declare a `throws SQLException` in the method, or surround the `while` code with a `try-catch`. --- `rs.getData(keyName)` is a `java.sql.ResultSet`, right? Verify if the `.getData(String)` method exists; because looks like it doesn't. Maybe you meant *getDat**e***? – CosmicGiant Dec 28 '15 at 17:00
  • @wero I don't know about other compilers, but the JDK's, used by NetBeans (8), ***is*** smart enough to not issue a warning. =) – CosmicGiant Dec 28 '15 at 17:10
  • @AlmightyR interesting. I tried with `javac` from JDK 8 but it complains. Doesn't NetBean use `javac`?. – wero Dec 28 '15 at 17:14
  • @wero AFAIK it uses `javac` from JDK, but it's not complaining here. Maybe you have an older version? I have `8u66` (`1.8.0_45-b15`) here. – CosmicGiant Dec 28 '15 at 17:17
  • @phillipe, sorry, i forgot to annotate that - that ResultSet is proprietary, and unfortunately, it's named the same as java.sql's ResultSet :/ it acts very similarly to java.sql.ResultSet, but is source agnostic, which is why it exists – liltitus27 Dec 28 '15 at 17:24
  • @wero, i understand that, but using Mars.1 and the latest build of Java 8, i am getting warnings. – liltitus27 Dec 28 '15 at 17:26

2 Answers2

3

Your intention to not blindly silence compiler warnings is honorable but your code example is a good example for a justified use of @SupressWarnings. Your code cannot fail, and still the compiler complains, therefore @SupressWarnings is the way to go.

Take a look at java.lang.Class#cast which has the same pattern as your example. It also suppresses the warning:

@SuppressWarnings("unchecked")
public T cast(Object obj) {
    if (obj != null && !isInstance(obj))
        throw new ClassCastException(cannotCastMsg(obj));
    return (T) obj;
}
wero
  • 32,544
  • 3
  • 59
  • 84
  • ok, that's what i expected. figured i'd see if the community knew of a way to handle that without suppression of the warning, but your example makes it fairly obvious that this approach is okay. thanks for the input! – liltitus27 Dec 28 '15 at 17:45
2

You can write:

            if (tclass.isAssignableFrom(o.getClass())) {
                result.add(tclass.cast(o));
            }

(See https://docs.oracle.com/javase/7/docs/api/java/lang/Class.html#cast(java.lang.Object).)

Edited to add: Incidentally, as erickson points out in a comment, you should probably be writing tclass.isInstance(o) instead of tclass.isAssignableFrom(o.getClass()): it's simpler and clearer, and it avoids a NullPointerException when o is null.

ruakh
  • 175,680
  • 26
  • 273
  • 307
  • that removes the warning and also accomplishes what i set out to do. so simple! – liltitus27 Dec 28 '15 at 17:46
  • 1
    @liltitus27: your comment makes me sad. you now have 2 native method calls instead of 1, and the cast is still there inside Class#cast. Out of sight, out of mind I guess. – wero Dec 28 '15 at 17:52
  • @wero why does that make you sad? :( i don't wanna make anyone sad. you yourself said that `Class#cast` does what I'm doing, so why not delegate the warning suppression to `java.lang.Class`? as long as i keep that call inside the `isAssignableFrom` check, i'm okay, right? or is your point that any future dev doesn't see this suppressed warning, and so isn't obviously aware that this is occurring? – liltitus27 Dec 28 '15 at 17:58
  • oh, and i suppose i'm duplicating checks this way, too, since `Class#cast`'s code performs safety checks that i've already determined are not needed. – liltitus27 Dec 28 '15 at 18:00
  • This is a good solution. It will prevent class casting errors in locations where no explicit cast occurs (like where items are consumed from the list). It's not an unsafe cast, like `add((T) o)` would be. Use `isInstance()` instead of `isAssignableFrom()`, though. – erickson Dec 28 '15 at 18:00
  • @erickson, are you sure? looking at http://stackoverflow.com/a/6983596/1322243, it seems to me that i would want to keep it using `isAssignableFrom`. what's your reasoning for not using that method? – liltitus27 Dec 28 '15 at 18:23
  • 1
    @liltitus27 Really? Why? I don't see anything in that answer to support your use of `isAssignableFrom()`. You have an object. You want to test whether it's an instance of a particular type. That's precisely the function of `isInstance()`. `isAssignableFrom()` is a roundabout way of doing the same thing, and that roundabout includes the possibility of a `NullPointerException`. – erickson Dec 28 '15 at 18:46
  • @erickson...whoa, did my asking a question on stackoverflow just piss you off? `isInstance` won't return `true` for superclasses or interfaces for which Class is applicable. which is why i chose to use `isAssignableFrom` – liltitus27 Dec 28 '15 at 19:22
  • additionally, i don't have an instance of `T`, i have a `Class` of type `T` – liltitus27 Dec 28 '15 at 19:28
  • 1
    @liltitus27: I don't think you've "pissed [erickson] off". I read his/her comment as expressing confusion rather than anger. (FWIW, I share that confusion. The answer that you've linked to explains that this use-case is the exact purpose of `isInstance`, and I don't see how you can be interpreting it differently.) – ruakh Dec 28 '15 at 19:30
  • @ruakh, ok, guess i got a lil defensive there, my bad. i'm having trouble understanding the difference between the two. i've read the javadocs, and the linked article, but evidently, i'm not discerning that difference adequately. – liltitus27 Dec 28 '15 at 19:32
  • @liltitus27: I've updated my answer to explain how you'd use `isInstance` -- hopefully that clarifies. – ruakh Dec 28 '15 at 19:35
  • 1
    @liltitus27 Yes, ruakh is right, I was not mad, just bewildered. The linked answer's passage, "... `obj` is an instance of `MyClass` or its subclasses..." means that `obj` "is-a" `MyClass`---it's either `MyClass` or something that implements or extends `MyClass`. In statically typed code you'd want to write: `if (o instanceof T) result.add((T) o);`, right? The `isInstance()` method is the reflective equivalent: it *will* return `true` when the target `Class` is an interface implemented by the argument, or is a super class of the argument. – erickson Dec 29 '15 at 06:00
  • @erickson thanks for the explanation and kinda spelling it out for me, sometimes it takes a minute for things to click. – liltitus27 Dec 29 '15 at 17:33