HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaModerate

Finding the Players with the highest score

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thewithfindingplayersscorehighest

Problem

Given a list of Players with score, I want to find the ones with the highest score. There can be multiple players with the same score. I'm doing it like this now:

class Player {
    final String name;
    final int score;

    Player(String name, int score) {
        this.name = name;
        this.score = score;
    }
}

class PlayerComparatorByScore implements Comparator {
    @Override
    public int compare(Player o1, Player o2) {
        return -Integer.compare(o1.score, o2.score);
    }
}

class PlayerUtil {
    static Collection getHighestScoringPlayers(Collection players) {
        List sortedPlayers = new ArrayList<>(players);
        Collections.sort(sortedPlayers, new PlayerComparatorByScore());

        Set highestScoringPlayers = new HashSet<>();
        Iterator iterator = sortedPlayers.iterator();
        Player highestScoringPlayer = iterator.next();
        highestScoringPlayers.add(highestScoringPlayer);

        while (iterator.hasNext()) {
            Player player = iterator.next();
            if (player.score == highestScoringPlayer.score) {
                highestScoringPlayers.add(player);
            } else {
                break;
            }
        }

        return highestScoringPlayers;
    }
}

public class PlayersSortedByScoreTest {
    @Test
    public void testSortingByScore() {
        Collection players = new HashSet<>();
        players.add(new Player("Alice", 3));
        players.add(new Player("Bob", 1));
        players.add(new Player("Mike", 3));

        Collection highestScoringPlayers = PlayerUtil.getHighestScoringPlayers(players);

        assertEquals(2, highestScoringPlayers.size());
        assertEquals(3, highestScoringPlayers.iterator().next().score);
    }
}


So this is pretty awkward... Is there a better way?

Solution

This is a terrible approach. Sorting the list (an \$O(n \log(n)\$) step) and then iterating through it (an \$O(n)\$ operation) to collect the highest scoring players is simply inefficient.

Instead, just iterate through the list in this way:

List playersList = new ArrayList<>(players);
Set highestScoringPlayers = new HashSet<>();
int maxScore = Integer.MIN_VALUE;
for (Player player : playersList) {
    maxScore = Math.max(maxScore, player.score)
}
for (Player player : playersList) {
    if (player.score == maxScore) {
        highestScoringPlayers.add(player);
    }
}
return highestScoringPlayers;


This uses 2 loops and the minimal amount of space (just enough for the original list and the highest scorers) and so is \$O(n)\$, which is better than your \$O(n \log(n))\$ solution.

If you would like to only use 1 loop, at the cost of some additional overhead here is the code:

List playersList = new ArrayList<>(players);
Set highestScoringPlayers = new HashSet<>();
int maxScore = Integer.MIN_VALUE;
for (Player player : playersList) {
    if (player.score >= maxScore) {
        if (player.score > maxScore) {
            maxScore = player.score;
            highestScoringPlayer.clear();
        }
        highestScoringPlayer.add(player);
    }
}

Code Snippets

List<Player> playersList = new ArrayList<>(players);
Set<Player> highestScoringPlayers = new HashSet<>();
int maxScore = Integer.MIN_VALUE;
for (Player player : playersList) {
    maxScore = Math.max(maxScore, player.score)
}
for (Player player : playersList) {
    if (player.score == maxScore) {
        highestScoringPlayers.add(player);
    }
}
return highestScoringPlayers;
List<Player> playersList = new ArrayList<>(players);
Set<Player> highestScoringPlayers = new HashSet<>();
int maxScore = Integer.MIN_VALUE;
for (Player player : playersList) {
    if (player.score >= maxScore) {
        if (player.score > maxScore) {
            maxScore = player.score;
            highestScoringPlayer.clear();
        }
        highestScoringPlayer.add(player);
    }
}

Context

StackExchange Code Review Q#60629, answer score: 10

Revisions (0)

No revisions yet.