-2

Trying to sort a list in descending order, so longest time first. This is my method I used few pages from here to make it correct however something in my code is wrong and it returns the list which is not so correct.

public static ArrayList<String> winnerIs(List<HP> hp){
        //System.out.println("1");
        int size = hp.size();
        //System.out.println(size);
        ArrayList<HP> listofWinner = new ArrayList<HP>();
        Map<String, Integer> map = new HashMap<String, Integer>();

        for(int i = 0; i < size; i++){
            listofWinner.add(hp.get(i));
            map.put(hp.get(i).getName(), hp.get(i).TD1());
            //System.out.println(hp.get(i).getName()+" "+hp.get(i).TD1());
        }
        //sort based on time
        ArrayList<String> keys = new ArrayList<String>(map.keySet());
        //System.out.println("---------------");
        /*for(int i = 0; i < keys.size(); i++){ 
            //wn.add(keys.get(i));
            System.out.println("here "+keys.get(i));
        }*/
        //System.out.println("---------------");


        ArrayList<String> wn = new ArrayList<String>();

        //System.out.println("---------------");
        for(int i = keys.size()-1; i >= 0; i--){    
            wn.add(keys.get(i));

        }
        return wn;
    }

here is what it reurns:

[team2, team1, team4, team3]

but it should be like this:

[team4, team3, team2, team1]

it doesn't matter if the time is equal, we just need the better time, I am not sure what part of the code is wrong.

even when I use this

ArrayList<Integer> s = new ArrayList<Integer>(map.values());
        Collections.sort(keys);
        //System.out.println("---------------");
        for(int i = 0; i < s.size(); i++){  
            //wn.add(keys.get(i));
            System.out.println("here "+s.get(i));
        }

the result are still not correct here is what it returns:

here 2
here 9
here 0
here 0

so I used one of the pages at stackoverflouw and I found this solution:

public static ArrayList<String> winnerIs(List<HumanPlayer> hp){
        //System.out.println("1");
        int size = hp.size();
        //System.out.println(size);
        ArrayList<HumanPlayer> listofWinner = new ArrayList<HumanPlayer>();
        Map<String, Integer> map = new HashMap<String, Integer>();

        for(int i = 0; i < size; i++){
            listofWinner.add(hp.get(i));
            map.put(hp.get(i).getName(), hp.get(i).getTimeDriver1());
            //System.out.println(hp.get(i).getName()+" "+hp.get(i).getTimeDriver1());
        }
        map.entrySet().stream()
        .sorted(Map.Entry.<String, Integer>comparingByValue().reversed()) 
        .limit(1000) 
        .forEach(System.out::println);

        return null;
    }

this returns the correct list but I am not sure what is this: .limit(1000) and also how can I equal this to a list, so I can return it.

  • 1
    You're actually doing reordering, not sorting. `HashMap` is an unordered map. Use `LinkedHashMap` instead, or sort the result list with `Collections.sort`. – Lyubomyr Shaydariv Jan 09 '17 at 18:18
  • You are putting name as the key and sorting the keyset() obtained later on. I do not know what TD1() is, but if it is the time that you want to sort with then you should be sorting values in your Map and not keys. – Amit Jan 09 '17 at 18:19
  • @Amit how can I do that should it be valueSet() instead of keySet()?? – Shervin Shemrani Jan 09 '17 at 18:21
  • if you implement Comparable in your HP class, then you can just sort the input List and then reverse the order, no Map necessary – geneSummons Jan 09 '17 at 18:28
  • this is highly unclear, can you cleanup your code, and title, wn is a list not a map –  Jan 09 '17 at 18:28
  • Possible duplicate of [Sort Map by value using java 8](http://stackoverflow.com/questions/29567575/sort-map-by-value-using-java-8) – Rogue Jan 09 '17 at 18:57
  • @Rogue thanks alot man, your comment helped and I get the correct answer! – Shervin Shemrani Jan 09 '17 at 19:36
  • no worries @ShervinShemrani, though check my answer for the part about map contracts (it'll help you understand why the differnent ones are used in this situation). – Rogue Jan 09 '17 at 19:40

3 Answers3

0

Assuming the TD1() method in your HP class is the value you want to sort against, and that you really want to use a Map to help you sort. I think you want something like this

Map<Integer, List<String>> map = new HashMap<Integer, List<String>>();
for (HP h : hp) {
    if (map.get(h.TD1() != null) {
        map.get(h.TD1()).add(h.getName());
    }
    else {
      List temp = new ArrayList<String>();
      temp.add(h.getName());
      map.put(h.TD1(), temp);
    }
}
ArrayList keys = Arrays.asList(map.getKeyset().toArray());
Collections.sort(keys);

for ( int i = keys.length() - 1; i >= 0; i--) {
    List<String> names = map.get(i);
    // print names
}
geneSummons
  • 907
  • 5
  • 15
  • No this is not good becasue the method getName is not a list here by using h.getName() it says that the get method should also change BTW i get so many type casting error using your solution. – Shervin Shemrani Jan 09 '17 at 18:55
0

You can use Java 8 for a nice sorting by value of the map:

Map<String, Integer> sorted = /* your map */.entrySet().stream()
        .sorted(Entry.comparingByValue()) //comparator for value, can reverse or use other
        .collect(Collectors.toMap(Entry::getKey, Entry::getValue,
                (e1, e2) -> { throw new IllegalArgumentException("Duplicate Key: " + e1.getKey()); },
                LinkedHashMap::new));

I chose to throw an exception for a duplicate key (the merging function, the 3rd argument of the Collectors#toMap), but you can also just return the first key found:

.collect(Collectors.toMap(Entry::getKey, Entry::getValue, (e1, e2) -> e1, LinkedHashMap::new));

The thing to keep in mind is the contracts that individual maps uphold. HashMap is an unsorted map and will not guaruntee iteration order (so sorting would be a fruitless endeavor), and a TreeMap is a SortedMap, but that contractually means it's sorted by key, not value. A LinkedHashMap will preserve iteration order, usually based on insertion (much like a List), and thus is usually what you want when you need to have a sorted map output.

Rogue
  • 11,105
  • 5
  • 45
  • 71
0

Check this psuedo-code out, I believe you would get overall idea looking at this.

Map<Integer,List<String> map = new HashMap<Integer,List<String>>();
for(HP hpObject:hp) {
 if(map.containsKey(hpObject.TD1())) {
    map.get(hpObject.TD1()).add(hpObject.getName());
 } else {
    List<String> names = new ArrayList<String>();
    names.add(hpObject.getName());
    map.put(hpObject.TD1(),names);
 }
}

    // To sort by keys
    TreeMap sortedByTD = new TreeMap(map);

    // Iterate over TreeMap and create the list of winners you need
    return result;enter code here
Amit
  • 1,111
  • 1
  • 8
  • 14
  • TreeMap is a SortedMap, which is contracted to sort by key (not value). It'd be better to utilize a `LinkedHashMap`, but I don't see you sorting anything at all in your answer. – Rogue Jan 09 '17 at 18:59
  • As you mentioned TreeMap is sorted hence when we create TreeMap from a HashMap as above, it would naturally sort it by Integer keys ( which are Timings). All you need to do is iterate it and create the final list of names to return. – Amit Jan 09 '17 at 19:04
  • Yes, but the op isn't using integer keys, he's using integer values. – Rogue Jan 09 '17 at 19:11
  • Yeah, that is what I mentioned in my comment to OP's post earlier. He should be using Time that is represented by Integer in his case as keys. – Amit Jan 09 '17 at 19:23
  • And if two people have the same time? There's just one person ever? This approach not only would break some design aspects but the answer itself overcomplicates things (and misses the first entry for a time, so a large amount would just be empty lists). – Rogue Jan 09 '17 at 19:30
  • That is why I have a List as a value for the time. Please check the if/else condition to add to existing list or add new list based on key availability. – Amit Jan 09 '17 at 19:31
  • precisely, when it doesn't contain the key, you make a new list and move on, completely ignoring the first `String` – Rogue Jan 09 '17 at 19:40
  • yeah sure. Thanks. Amended the pseudo-code. I hope the OP has got the point though. – Amit Jan 09 '17 at 19:49