views:

134

answers:

6

Hello everybody,

I have an old personal project written in Java 1.4 that I am porting to 1.5 (in which I am a still newbie) for version 2. Besides adding new features to it and refactoring code, I am also migrating to generic collections, annotations etc.

There is a particular piece of code that I don't know how to change to 1.5 and don't want to add a @SupressWarnings on it. It goes like this:

if (value instanceof Comparable) {
  isMatch = (((Comparable) value).compareTo(selectedValue) == 0);
} else {
  //fallback to equals
  isMatch = selectedValue.equals(value);
}

It is just a simple comparison with an extensive match in compareTo() or defaulting to plain equals() if not a Comparable type.

I am getting a : Comparable is a raw type. References to generic type Comparable<T> should be parameterized warning.

How do I modify the above code to 1.5 and clear the warning. Or is there no choice but to add @SuppressWarnings?

Thanks!

A: 

The warning is because there is no generic type used when you use Comparable.

Whatever class value is you are going to want to have that class implement Comparable<Value> where the generic type Value is whatever class value is. Then you can just call value.compareTo(selectedValue) with no need for the line value instanceof Comparable.

tkeE2036
Yes, but the thing is this is used for elements in a collection with <?> so I have in there all kinds of objects which implement different Comparable<Value1>, Comparable<Value2> etc. It is not always one type: Value. I was thinking Comparable<?> but that gives me an error, no more warning.
Elena
`Value` should be the class of `selectedValue` in this example.
grddev
Also, you would still need the runtime-check for `instanceof`.
grddev
A: 

@SuppressWarnings (2 p's) doesn't work particularly well in old Java 5.0 (There was no Java 1.5) however, since Java 5.0 has been end of life'd for nearly a year, I suggest you use Java 6 which doesn't suppress warnings a little better.

The important thing to realise is that its just a warning. Even the Collections library cannot be compiled without warnings and it doesn't appear that the designer intend that you can always write code which produces no warning at all.

Peter Lawrey
Could you elaborate on how `@SuppressWarnings` is better in Java 6?
grddev
There were problems with Java 5.0 honouring this annotation depending on where you put it. i.e. it didn't stop the warning. I haven't used Java 5.0 for almost four years and it could have improved since then, but as its no longer freely supported I doubt it.
Peter Lawrey
A: 

You can fix the warning just by making sure that the class of value extends comparable.

class ClassOfValueUsingCompareTo implements Comparable{
       //class definition
}

Source: http://forums.sun.com/thread.jspa?threadID=785803

If you do this you might have to change the logic by perhaps introducing more abstraction by 1)Using an interface to provide for polymorphism. 2)encapsulating the isMatch operation in a method that is defined on the polymorphic classes.

public interface ValueInterface{
      //interface definition
      public boolean isMatch();
}

class ClassOfValueUsingCompareTo implements ValueInterface,Comparable{
       //class definition
      public boolean isMatch(){
          //use compareto
       }
} 
class ClassOfValueUsingEquals  implements ValueInterface{
       //class definition
      public boolean isMatch(){
          //use equals
       }
}

As a side note: Problems with using instanceOf operator http://stackoverflow.com/questions/2790144/avoiding-instanceof-in-java

Hope this answers your question.

Shishya
you are still using a raw type
newacct
@newacct Thanks for pointing that out.
Shishya
+1  A: 

I don't see a way to write this code without a warning. You would have to cast value to Comparable<? super Value>, which (because it specifies a restriction on the type parameter) is an unchecked cast, which also results in a warning.

I recommend suppressing the warning, as the code will cleanly fail even if value were to implement Comparable with an unsuitable type parameter (serves them right if they implement Comparable, but can't compare to instances of the same class ...).

meriton
+1  A: 

I would rewrite this piece of code like this:

if (value instanceof Comparable<?>) { // <-- notice <?>
  // Declare local var and cast it here
  // You will incur warning, but it's localized to this one instance only
  //
  // Also not that this cast is safe as it matches the signature of Comparable
  // in Java 1.4
  @SuppressWarnings( "unchecked" )
  Comparable<Object> comp = (Comparable<Object>)value;

  // Now you can use compareTo
  isMatch = (comp.compareTo(selectedValue) == 0);
} else {
  //fallback to equals
  isMatch = selectedValue.equals(value);
}
Alexander Pogrebnyak
Why not just add `@SuppressWarnings` and keep the rest of the code as is?
grddev
@grddev. Because it can mask a really hideous condition in the scope that it's applied to. It's just good housekeeping.
Alexander Pogrebnyak
+2  A: 

Specifically, you get the warning, because you need to qualify the Comparable with a type argument in the cast. Such that you have ((Comparable<Type>)value), where Type is (a basetype of) the type of selectedValue.

However, even if you do this, you will still get a warning. This time, the problem is that the <Type> "goes away" when compiling, which means that there is no way to ensure that the class is actually Comparable<Type>. You can only check whether the object is Comparable. Consider the following test application:

class Type{
  public int x = 0;
}

class Other implements Comparable {
  public int compareTo(Type obj) {
    return obj.x;
  }
}

class Test {
  public static void main(String[] args) {
    Other obj1 = new Other();
    Object value = obj1;
    String selectedValue = "";
    if (value instanceof Comparable) {
       System.out.println(((Comparable<String>) value).compareTo(selectedValue));
    }
  }
}

The code compiles with a warning, because while value is Comparable it cannot be compared with Strings. The latter is still preferable, since you can at least check that the argument is of the "expected" type.

In the end, you will need to @SuppressWarnings.

Note that if you want it to work with any class. Then you can just as well leave it as is (or use Comparable<Object> as they are (in this case) equivalent). With the addition of @SuppressWarnings, obviously.

grddev