0

I am using Java 1.7 and Eclipse Juno.

I have a list of objects that can be sorted by any one of the object's properties. To accomplish the sort I put into a map the list index of each object with the value of the property that is selected as the sort. The map is then sorted by value and then a new list assembled by looking up the objects in the original list and adding them to a new list.

There are other ways to go about sorting a list by different properties, but this is the approach taken by some code I am cleaning up, and I can't change it too drastically (it is part of a single method many thousands of lines long, with lots of what are effectively global variables. I'm refactoring as time allows, but there are no tests, so it's slow going).

I want to avoid explicit typecasting, raw types, and unchecked cast/invocations.

The program below illustrates the idea. It works, but produces raw types and unchecked conversion warnings. I could suppress the warnings, but I would prefer if there were a way to declare it such that it does not generate a warning and also is compatible with the sortByValue() function.

Also, I noticed that when I write:

Map<Integer,Comparable> b = sortByValue(map);
for( Integer index : b.keySet()){
    output.add(animals.get(index));    
}

The type of .keySet() is the expected Set<Integer>. But if I write:

for( Integer index : sortByValue(map).keySet()){
    output.add(animals.get(index));    
}

Then the type of .keySet() is unparameterized Set, and I get a type mismatch error. I can use type Object and explicit cast index to Integer, but I don't understand why the return type of keySet() should be different.

package my.sandbox;
import java.util.*;

public class Main {

    public static void main(String[] args) {
        try {
            test();
        } catch (Exception e) {
            e.printStackTrace();
        }
    }

    // https://stackoverflow.com/a/2581754/145446
    public static <K, V extends Comparable<? super V>> Map<K, V> sortByValue(Map<K, V> map) {
        List<Map.Entry<K, V>> list = new LinkedList<Map.Entry<K, V>>(map.entrySet());
        Collections.sort(list, new Comparator<Map.Entry<K, V>>() {
            public int compare(Map.Entry<K, V> o1, Map.Entry<K, V> o2) {
                return (o1.getValue()).compareTo(o2.getValue());
            }
        });

        Map<K, V> result = new LinkedHashMap<K, V>();
        for (Map.Entry<K, V> entry : list) {
            result.put(entry.getKey(), entry.getValue());
        }
        return result;
    }

    static class Animal {
        String Name;
        Double Weight;
        Integer Age;
        Animal(String n, Double w, Integer a){
            Name = n;
            Weight = w;
            Age = a;
        }
        public String toString(){
            return String.format("%10s : %6.1f   %d", Name, Weight, Age);
        }
    }

    private static void test() {
        List<Animal> animals = new LinkedList<Animal>();
        animals.add(new Animal("Spot", 35.4, 5));
        animals.add(new Animal("Rover", 47.2, 3));
        animals.add(new Animal("Phydeaux", 55.2, 4));


        int sortOption = new Random(System.currentTimeMillis()).nextInt(3);
        // ** Two 'raw types' warnings here
        Map<Integer, Comparable> map = new HashMap<Integer, Comparable>();

        for( Integer i =0; i<animals.size(); i++)
        {
            switch (sortOption) {
                case 0 : 
                    map.put(i, animals.get(i).Name);
                    break;
                case 1 :
                    map.put(i, animals.get(i).Weight);
                    break;
                case 2 :
                    map.put(i, animals.get(i).Age);
                    break;
            }
        }

        List<Animal> output = new LinkedList<Animal>();

        // ** One each 'raw types' and unchecked conversion warnings here
        for( Object index : sortByValue(map).keySet()){
            output.add(animals.get((Integer)index));    
        }

        for( Animal s : output)
            System.out.println(s);
    }
}

Edit:

After exploring a number of options I began to wonder why the map was being used at all. I just need to compare objects in different ways, so why not just build an appropriate Comparator, then sort the list directly?

private static void test() {
    List<Animal> animals = new LinkedList<Animal>();
    animals.add(new Animal("Spot", 35.4, 5));
    animals.add(new Animal("Rover", 47.2, 3));
    animals.add(new Animal("Phydeaux", 55.2, 4));

    final int sortOption = new Random(System.currentTimeMillis()).nextInt(3);

    // Build a comparator for the specified sort
    Collections.sort(animals, new Comparator<Animal>() {
        public int compare(Animal o1, Animal o2) {
            switch (sortOption){
            case 0: 
                return (o1.Name).compareTo(o2.Name);
            case 1: 
                return (o1.Weight).compareTo(o2.Weight);
            case 2: 
                return (o1.Age).compareTo(o2.Age);
            }
            return 0;
        }});

    for( Animal s : animals)
        System.out.println(s);
}
Community
  • 1
  • 1
DaveK
  • 709
  • 7
  • 21

1 Answers1

1

The first raw types warnings are telling you what's going on: as soon as you try to use a raw Comparable, all bets are off. Your best bet is to move the declaration of map into a switch statement, rather than trying to switch inside the for loop and shoehorn names, weights, and ages into the same type.

Louis Wasserman
  • 191,574
  • 25
  • 345
  • 413
  • I don't understand why that would affect the result of `keySet`, though, since this returns `Set` where `K` is the key type, and the value is the one where he's using `Comparable`. I don't have all the rules about generics down, though. – ajb Nov 15 '13 at 23:43
  • It looks like you're right, but I don't understand why. When I make the map `Map` (and comment out the loop) the warnings go away. – ajb Nov 15 '13 at 23:47
  • I suspect it has to do with the weird mechanics of erasure, where if one thing is erased, everything gets erased. I'm not sure how that manifests in this specific context, however. – Louis Wasserman Nov 15 '13 at 23:48
  • I don't want to use the raw type, but I have not been able to find a way to express the type of `map` in a way that allows me to put elements of various types and also call `sortByValue(map)`. I'm not sure what you mean by moving the declaration of `map` into a switch; if I do that, then I have to have a call to `sortByValue` in each case. Perhaps sorting a hashmap is the wrong approach for this problem. – DaveK Nov 18 '13 at 17:48
  • @DaveK: Pretty much what I'm saying is that putting elements of various types into the same type of map is the problem; that's the part of your design you probably ought to fix. If you do insist on that approach, then accepting a warning is the proper solution: you're insisting on doing something that the compiler can't prove is safe, and `@SuppressWarnings` is the right way of saying that you're sure, even if the compiler isn't. – Louis Wasserman Nov 18 '13 at 19:04
  • What I was looking for was a way to declare `map` to express that it will contain exactly one type 'T', and that 'T' supports `Comparable`, so that I can pass the map to the sort routine without a warning. That would require casting when `put`ing objects in the map, which would be ok. It doesn't look like there is a way to express such a type for `map`. – DaveK Nov 18 '13 at 22:14