Avoiding warnings about generics: 2 questions

[Scott W Gifford]

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.

 

[Thomas Hawtin]

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();
}
}
}

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

 

[Roedy Green]

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();
}
}
}

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

 

[Chris Uppal]

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

 

[Scott W Gifford]

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.

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);
}
}

 

[Chris Uppal]

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

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