3
\$\begingroup\$

I am solving the following Hackerrank problem: Climbing the Leaderboard.

The problem statement:

Alice is playing an arcade game and wants to climb to the top of the leaderboard and wants to track her ranking. The game uses Dense Ranking, so its leaderboard works like this:

  • The player with the highest score is ranked number 1 on the leaderboard.
  • Players who have equal scores receive the same ranking number, and the next player(s) receive the immediately following ranking number.

For example, the four players on the leaderboard have high scores of 100, 90, 90, and 80. Those players will have ranks 1, 2, 2, and 3, respectively. If Alice's scores are 70, 80 and 105, her rankings after each game are 4th, 3rd and 1st.

Function Description

Complete the climbingLeaderboard function in the editor below. It should return an integer array where each element res[j] represents Alice's rank after the jth game.

climbingLeaderboard has the following parameter(s):

scores: an array of integers that represent leaderboard scores alice: an array of integers that represent Alice's scores

Input Format

The first line contains an integer n, the number of players on the leaderboard. The next line contains n space-separated integers scores[i], the leaderboard scores in decreasing order. The next line contains an integer, m, denoting the number games Alice plays. The last line contains m space-separated integers alice[j], the game scores.

Constraints

  • 1 <= n <= 2 * 10^5
  • 1 <= m <= 2 * 10^5
  • 0 <= scores[i] <= 10^9 for 0 <= i < n
  • 0 <= alice[j] <= 10^9 for 0 <= j < m

The existing leaderboard, scores, is in descending order. Alice's scores, alice, are in ascending order.

Subtask

For 60% of the maximum score:

  • 1 <= n <= 200
  • 1 <= m <= 200

Output Format

Print m integers. The jth integer should indicate Alice's rank after playing the jth game.


My solution is failing to pass 4 test cases due to timeout. The code is as follows:

 import java.util.*;
    
    public class Main {
        public static void main(String[] args) {

            Scanner scanner = new Scanner(System.in);

            int totalPlayers = scanner.nextInt();
            ArrayList<Integer> playerScores = new ArrayList<>();
            for(int i = 0; i < totalPlayers; i++) {
                playerScores.add(scanner.nextInt());
            }
    
            int aliceTotalScores = scanner.nextInt();
            ArrayList<Integer> aliceScores = new ArrayList<>();
            for(int i = 0; i < aliceTotalScores; i++) {
                aliceScores.add(scanner.nextInt());
            }
    
            List<Integer> finalScores = new ArrayList<>(playerScores);
            for(int i = 0; i < aliceTotalScores; i++) {
                finalScores.add(aliceScores.get(i));
                finalScores.sort(Collections.reverseOrder());
                Set<Integer> set = new LinkedHashSet<>(finalScores);
                finalScores.clear();
                finalScores.addAll(set);
                System.out.println(finalScores.indexOf(aliceScores.get(i)) + 1);
            }
        }
    }

Any suggestions on how to improve the solution or some general direction as to where I am making a mistake would be appreciated. Thanks in advance.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ Use a binary search to determine where on the leader board Alice's score belongs. With up to 10 to the ninth scores on the leader board, any other search method is too slow. \$\endgroup\$ Jun 19 '20 at 17:40
1
\$\begingroup\$

I have some suggestions to make the code cleaner, not faster.

Always try to pass the size of the maximum size to the Collection / Map when known

The ArrayList has a default size of 10 elements, if you have more elements, the list will have to resize its internal pool. By setting the size, you can prevent the resize and make your code faster.

//[...]
int totalPlayers = scanner.nextInt();
ArrayList<Integer> playerScores = new ArrayList<>(totalPlayers);
//[...]
int aliceTotalScores = scanner.nextInt();
ArrayList<Integer> aliceScores = new ArrayList<>(aliceTotalScores);
//[...]

Always use the base class / interface in the left part of the variable when possible

By setting the java.util.List interface in the variable part, this could make the code more refactorable in the future, since you could easily change the list type without changing everything (inheritance).

Before

 ArrayList<Integer> playerScores = new ArrayList<>(totalPlayers);

After

 List<Integer> playerScores = new ArrayList<>(totalPlayers);

Replace the for loop with an enhanced 'for' loop

In your code, you don’t actually need the index provided by the loop, you can the enhanced version.

Before

for (int i = 0; i < aliceTotalScores; i++) {
   finalScores.add(aliceScores.get(i));
   finalScores.sort(Collections.reverseOrder());
   Set<Integer> set = new LinkedHashSet<>(finalScores);
   finalScores.clear();
   finalScores.addAll(set);
   System.out.println(finalScores.indexOf(aliceScores.get(i)) + 1);
}

After

for (Integer aliceScore : aliceScores) {
   finalScores.add(aliceScore);
   finalScores.sort(Collections.reverseOrder());
   Set<Integer> set = new LinkedHashSet<>(finalScores);
   finalScores.clear();
   finalScores.addAll(set);
   System.out.println(finalScores.indexOf(aliceScore) + 1);
}

Extract some of the logic to methods.

To remove some of the code of the main method, you could move both list instantiation (playerScores & aliceScores) to methods. Since the logic is similar, you could make only one method to read the inputs to a list and reuse the same for both cases.

public static void main(String[] args) {
   List<Integer> playerScores = getPlayerScores(scanner);
   List<Integer> aliceScores = getAliceScores(scanner);
}

private static List<Integer> getAliceScores(Scanner scanner) {
   return getIntegers(scanner);
}

private static List<Integer> getPlayerScores(Scanner scanner) {
   return getIntegers(scanner);
}


private static List<Integer> getIntegers(Scanner scanner) {
   int size = scanner.nextInt();
   List<Integer> integers = new ArrayList<>(size);
   for (int i = 0; i < size; i++) {
      integers.add(scanner.nextInt());
   }
   return integers;
}

or

public static void main(String[] args) {
   List<Integer> playerScores = getIntegers(scanner);
   List<Integer> aliceScores = getIntegers(scanner);
}

private static List<Integer> getIntegers(Scanner scanner) {
   int size = scanner.nextInt();
   List<Integer> integers = new ArrayList<>(size);
   for (int i = 0; i < size; i++) {
      integers.add(scanner.nextInt());
   }
   return integers;
}

Refactored code

public static void main(String[] args) {
   Scanner scanner = new Scanner(System.in);
   List<Integer> playerScores = getPlayerScores(scanner);
   List<Integer> aliceScores = getAliceScores(scanner);
   List<Integer> finalScores = new ArrayList<>(playerScores);

   for (Integer aliceScore : aliceScores) {
      finalScores.add(aliceScore);
      finalScores.sort(Collections.reverseOrder());
      Set<Integer> set = new LinkedHashSet<>(finalScores);
      finalScores.clear();
      finalScores.addAll(set);
      System.out.println(finalScores.indexOf(aliceScore) + 1);
   }
}

private static List<Integer> getAliceScores(Scanner scanner) {
   return getIntegers(scanner);
}

private static List<Integer> getPlayerScores(Scanner scanner) {
   return getIntegers(scanner);
}


private static List<Integer> getIntegers(Scanner scanner) {
   int size = scanner.nextInt();
   List<Integer> integers = new ArrayList<>(size);
   for (int i = 0; i < size; i++) {
      integers.add(scanner.nextInt());
   }
   return integers;
}
\$\endgroup\$
-1
\$\begingroup\$

The below solution is simpler and it should pass all the test cases. You don't need the sorting techniques to solve this. Your code is quite complicated as you did a sort in each loop which is not needed at all. Here what we can do instead:

1- Use Set to remove duplicate scores

2- Use LinkedHashSet to keep the set in order state

3- Save the Set again to a List which helps us to get the index of each score

Refactor code

public static void main(String[] args) {
    Scanner scanner = new Scanner(System.in);
    List<Integer> playerScores = getPlayerScores(scanner);
    List<Integer> aliceScores = getAliceScores(scanner);

    Set<Integer> uniqueScores = new LinkedHashSet<>(playerScores);
    playerScores = new ArrayList<>(uniqueScores);

    int i = playerScores.size() - 1;
    for (Integer oneScore : aliceScores) {
        while (i >= 0) {
            if (oneScore < playerScores.get(i)) {
                System.out.println(i + 2);
                break;
            }
            i--;
        }
        if (i < 0) {
            System.out.println(1);
        }
    }
}

private static List<Integer> getAliceScores(Scanner scanner) {
    return getIntegers(scanner);
}

private static List<Integer> getPlayerScores(Scanner scanner) {
    return getIntegers(scanner);
}


private static List<Integer> getIntegers(Scanner scanner) {
    int size = scanner.nextInt();
    List<Integer> integers = new ArrayList<>(size);
    for (int i = 0; i < size; i++) {
        integers.add(scanner.nextInt());
    }
    return integers;
}
\$\endgroup\$
3
  • 5
    \$\begingroup\$ Welcome! This is code-review, not post-your-solution. Please explain why this works and the main take-away for the OP. \$\endgroup\$ Mar 3 at 13:01
  • 4
    \$\begingroup\$ Welcome to Code Review! You have presented an alternative solution, but haven't reviewed the code. Please edit to show what aspects of the question code prompted you to write this version, and in what ways it's an improvement over the original. It may be worth (re-)reading How to Answer. \$\endgroup\$ Mar 3 at 13:22
  • \$\begingroup\$ Much better for the edit, but what about the parameters to climbingLeaderboard() does (supposedly) make it work? Don't write, never publish/commit undocumented/uncommented code! \$\endgroup\$
    – greybeard
    Mar 3 at 16:56

This site is temporarily in read only mode and not accepting new answers.

Not the answer you're looking for? Browse other questions tagged .