Avoiding warnings about generics: 2 questions

Thread view:

Scott W Gifford - 16 Dec 2005 01:01 GMT

I've got two generics-related warnings I can't figure out how to fix.

The first one is implementing a simple Comparator class for Object, to
sort an array that will mostly be Comparable objects, but may have
some non-Comparable ones thrown in, in which case I don't care about
the order of those. I've got this:

private final class CompHelper implements Comparator<Object> {
public int compare(Object o1, Object o2) {
if (o1 instanceof Comparable && o2 instanceof Comparable) {
return ((Comparable)o1).compareTo((Comparable)o2);
} else {
return o1.hashCode() - o2.hashCode();
}
}
}

which gives me:

Type safety: The method compareTo(Object) belongs to the raw type
Comparable. References to generic type Comparable<T> should be
parameterized

The second is taking a parameterized Class as a parameter, then
creating an instance of that class:

public Map<String,String> Example(Class<? extends Map> cacheClass) throws Exception {
return cacheClass.newInstance();
}

giving:

Type safety: The expression of type capture-of ? extends Map needs
unchecked conversion to conform to Map<String,String>

I've added @SuppressWarnings("unchecked") to these, but I'd rather fix
them The Right Way.

Any ideas?

Thanks!

---ScottG.

Reply to this Message

Thomas Hawtin - 16 Dec 2005 01:35 GMT

> I've got two generics-related warnings I can't figure out how to fix.
>
[quoted text clipped - 12 lines]
> }
> }

It's a nonsense class. You can tell if two objects are comparable, but
not if they are comparable to each other. So if statement is just pointless.

Thank goodness generics are there to point out the error of your ways. :)

The hashCode fall back is also a bit of a disaster. Hash codes are not
unique. Even if they were subtracting one int from another does not a
compare method make. The int can overflow. Stick to -1, 0 and 1.

> The second is taking a parameterized Class as a parameter, then
> creating an instance of that class:
>
> public Map<String,String> Example(Class<? extends Map> cacheClass) throws Exception {
> return cacheClass.newInstance();
> }

You would need something like:

public Map<String,String> newExample(
Class<? extends Map<String, String>> cacheClass
) throws ...lots... {

And that probably isn't a good idea. Call new instance on the
constructor of a more meaningful class.

Tom Hawtin

Signature

Unemployed English Java programmer
http://jroller.com/page/tackline/

Reply to this Message

Roedy Green - 16 Dec 2005 03:31 GMT

On Thu, 15 Dec 2005 20:01:30 -0500, Scott W Gifford
<gifford@umich.edu> wrote, quoted or indirectly quoted someone who
said :

> private final class CompHelper implements Comparator<Object> {
> public int compare(Object o1, Object o2) {
[quoted text clipped - 5 lines]
> }
> }

here is an example quoted from
http://mindprod.com/jgloss/comparator.html

The main difference I notice is you made your class private. You
can't have a private top level class.

class Numerically implements Comparator<Pair> // <--- note <Pair>
{
/**
* Compare two Pairs. Callback for sort. DESCENDING numerically,
ascending by name
* effectively returns b-a;
* @return +1, b>a, 0 a=b, -1 b<a, case insenstive
*/
public final int compare( Pair a, Pair b ) // <--- note Pair not
Object
{
int diff = b.count - a.count; // <--- note lack of Casts
if ( diff == 0 )
{
return a.filename.compareToIgnoreCase( b.filename );
}
else
{
return diff;
}
} // end compare

Signature

Canadian Mind Products, Roedy Green.
http://mindprod.com Java custom programming, consulting and coaching.

Reply to this Message

Chris Uppal - 16 Dec 2005 09:13 GMT

> public int compare(Object o1, Object o2) {
> if (o1 instanceof Comparable && o2 instanceof Comparable) {
> return ((Comparable)o1).compareTo((Comparable)o2);
> } else {
> return o1.hashCode() - o2.hashCode();
> }

Besides the two issues that Thomas has already raised with this, I believe that
this is logically unsound and may fail (throw exceptions or go into an infinite
loop) sporadically at runtime. Sorting requires that the comparison operation
is transitive:

a (<) b
and b (<) c
implies a (<) c

which is not guaranteed by the above implementation of compare() since a given
Comparable might compare low when compared with another Comparable (of the same
comparability-class) but high when compared with non-comparable. E.g. a
String-like-object, "a", might compare lower than almost any other
String-like-object, but the object itself might happen to have a hashCode() of
0xFFFFFFFF.

The only way to fix this is to make all non-comparables Compare lower than all
Comparables (or the other way around).

You might also want to consider the possibility that the objects in question
have customised hashCode()s (which might be slow or otherwise unsuitable for
this purpose), and instead use the system's identity hash code.

-- chris

Reply to this Message

Scott W Gifford - 16 Dec 2005 20:49 GMT

>> public int compare(Object o1, Object o2) {
>> if (o1 instanceof Comparable && o2 instanceof Comparable) {
[quoted text clipped - 15 lines]
> Comparable might compare low when compared with another Comparable (of the same
> comparability-class) but high when compared with non-comparable.

Ah, that's true, I hadn't thought of that. Thanks!

[...]

> The only way to fix this is to make all non-comparables Compare
> lower than all Comparables (or the other way around).

I think your strategy will work; see the implementation at the end of
this message.

But, it still doesn't solve my original problem: is it possible to get
rid of the warning without using @SuppressWarnings? The line with
compareTo() warns:

Type safety: The method compareTo(Object) belongs to the raw type
Comparable. References to generic type Comparable<T> should be
parameterized

Thanks!

---Scott.

import java.util.*;

public final class CompHelper implements Comparator<Object> {
     @SuppressWarnings("unchecked")
     public int compare(Object o1, Object o2) {
         if (o1 instanceof Comparable && o2 instanceof Comparable) {
             try {
                 return ((Comparable)o1).compareTo((Comparable)o2);
             } catch (ClassCastException e) { /* do nothing */ }
             // They must not be mutually Comparable, so let's call them equal.
             return 0;
         } else if (o1 instanceof Comparable)
             return -1;
         else if (o2 instanceof Comparable)
             return 1;
         else
             return 0;
     }

     public static void main(String[] args) {
//ArrayList has a better toString() than a regular array.
         ArrayList<Object> list = new ArrayList<Object>(Arrays.asList((new Object[] {
                 "string1",
                 new Object(),
                 "string2",
                 new Object(),
                 "string3",
                 new Object(),
         })));
         System.out.println(list);
         Collections.sort(list,new CompHelper());
         System.out.println(list);
     }
}

Reply to this Message

Chris Uppal - 17 Dec 2005 12:29 GMT

> But, it still doesn't solve my original problem: is it possible to get
> rid of the warning without using @SuppressWarnings? The line with
[quoted text clipped - 3 lines]
> Comparable. References to generic type Comparable<T> should be
> parameterized

Well, you /can/ get rid of the warning by changing the cast from (Comparable)o1
to (Comparable<Object>)o1. And, in fact, I can't think of an actual problem
that it would cause. It's still /very/ odd code, though. For instance in your
example, you are asking whether String implements Comparable<Object> (which it
does -- after erasure), and forcing it to use the compare(Object) method which
String doesn't even /declare/ !

I think you would probably be better off accepting that the type checker is
trying to tell you that your design is broken, at least with respect to the new
meaning of Comparator<T>. That means you should either change your design to
properly genericsify the whole thing, or decide that you want/need the old
meaning of Comparator. In the later case you will avoid all the generic forms,
and then just live with the warnings from the compiler (or use -source 1.4).
By and large, and without knowing what you are really trying to do, I lean
towards the former option myself.

-- chris

Reply to this Message

Avoiding warnings about generics: 2 questions

Free Magazines