0

I am using Comparator to compare files by size, but when I tried to compile my code i got warning: "java uses unchecked or unsafe operations". I put my code into comments and than the program worked, so I think is the problem with sorting in Comparator class. Here is my code:

public class size implements Comparator {

    @Override
    public int compare(Object o1, Object o2) {

        long s1 = ((Class)o1).getSize();
        long s2 = ((Class)o2).getSize();

        if (s1 > s2){
            return 1;
        }
        else if (s1 < s2){
            return -1;
        }
        else {
            return 0;
        }
    }
}
Joachim Sauer
  • 302,674
  • 57
  • 556
  • 614
Erik
  • 335
  • 2
  • 9
  • 19

7 Answers7

3

These two lines contain unsafe casts:

long s1 = ((Class)o1).getSize();
long s2 = ((Class)o2).getSize();

The unsafe cast is this expression: (Class)o1, you cast o1 which is an object to a Class, without previously checking that o1 is an instance of Class

Your program works well if you provide instances of Class to the method. The problem is that you can't make sure that nobody calls yur method with an object that is not a Class.

You should implement a type safe comparator, provided that your java version is newer that 1.4.

public class size implements Comparator<Class> {   
    @Override
    public int compare(Class o1, Class o2) {
        // compare the two class objects
nakosspy
  • 3,904
  • 1
  • 26
  • 31
  • But if i remove this casts the getter method can't be resolved. – Erik May 23 '13 at 13:29
  • @Erik, btw is it java.lang.Class objects the ones that you are trying to compare? Because there is no getSize() method in java.lang.Class. – nakosspy May 23 '13 at 13:42
2

declare your Comparator as a Comparator<File> and replace the compare method with

compare(File o1, File o2)

EDIT : Or Comparator<Class> if you are comparing classes. This is what you seem to do

Arnaud Denoyelle
  • 29,980
  • 16
  • 92
  • 148
2

I think there are errors in the code. You should do something like this :

 public static void main(String[] args) {
    File parentFile = new File("path to your parent file");
    File[] files = parentFile.listFiles();
    Arrays.sort(files, new Comparator<File>() {
    @Override
    public int compare(File o1, File o2) {
        return new Long(o1.length()).compareTo(o2.length());
    }
});
yavuzkavus
  • 1,268
  • 11
  • 17
0

You should use o1.getClass() instead of casting

0

First, you are using Comparator as a raw type. This is wrong, add a type argument to it and then implement compare(File, File).

Second, the performance of your Comparator will be terrible because length results in a native system call to find out the size of the file.

To solve the performance issue you'll need to write a wrapper class for files. It can implement Comparable directly:

public class FileBySize implements Comparable<FileBySize> {
   private final File f;
   private final Long size;
   public FileBySize(File f) { this.f = f; this.size = f.length(); }
   @Override public int compareTo(FileBySize other) {
     return this.size.compareTo(other.size);
   }
}
Marko Topolnik
  • 195,646
  • 29
  • 319
  • 436
0

This is right. Comparator is a parametrized interface, i.e. class that is defined with generics.

This is the way you should implement your comparator to avoid both warnings and a chance that ClassCasetException is thrown:

public class SizeComparator implements Comparator<Object> {
......
}

Your comparator is kind of something special. It works with any object. This is a reason that I wrote Comparator<Object> here. In most cases you'd define class parameter more specifically and use the same class in compare() method, for example

public class SizeComparator implements Comparator<String> {
     public int compare(String s1, String s2) {
         .......
     }
}

BTW in your case you can also define your comparator as following:

public class SizeComparator implements Comparator<T extends Object> {
     public int compare(T o1, T o2) {
         .......
     }
}

And pay attention on the fact that I changed name of your class. It is important to follow wide knows naming conventions.

AlexR
  • 114,158
  • 16
  • 130
  • 208
-1
long s1 = ((Class)o1).getSize();

Is where it is complaining. I think this fixes it:

if(o1 instanceof Class)  
{  
     long s1 = ((Class)o1).getSize();
}  

Essentially you have not guaranteed that o1 is a Class

Woot4Moo
  • 23,987
  • 16
  • 94
  • 151