How to change my code to make it more efficient?

I wouldn't say this is necessarily more efficient or even better but it is a little more concise. It's major elements are.

• use Lambdas to decide the winner based on the chosen move.
• use a map to call the proper Lambda based on the user's move. The lambda then evaluates the two moves to decide the outcome.
• for simplicity, moves are selected by number

Of course, the important thing is that your code works.

import java.util.List; import java.util.Map; import java.util.Random; import java.util.Scanner; import java.util.Set; import java.util.function.Function; public class RockPaperScissors { final static int PAPER = 1; final static int ROCK = 2; final static int SCISSORS = 3; // the next two declarations allows the previous three to take on any values. private Set<Integer> allowedMoves = Set.of(PAPER, ROCK, SCISSORS); private List<String> moves = List.of("PAPER", "ROCK", "SCISSORS"); private String ROCK_WINS_MSG = "Rock crushes scissors"; private String SCISSORS_WINS_MSG = "Scissors cuts paper"; private String PAPER_WINS_MSG = "Paper covers rock"; private String COMPUTER_WINS = ", computer wins!"; private String YOU_WIN = ", you win!"; private Function<Integer, String> CHECK_PAPER = (c) -> c == PAPER ? "It's a tie!" : c == ROCK ? PAPER_WINS_MSG + YOU_WIN : SCISSORS_WINS_MSG + COMPUTER_WINS; private Function<Integer, String> CHECK_ROCK = (c) -> c == ROCK ? "It's a tie!" : c == SCISSORS ? ROCK_WINS_MSG + YOU_WIN : PAPER_WINS_MSG + COMPUTER_WINS; private Function<Integer, String> CHECK_SCISSORS = (c) -> c == SCISSORS ? "It's a tie!" : c == PAPER ? SCISSORS_WINS_MSG + YOU_WIN : ROCK_WINS_MSG + COMPUTER_WINS; private Map<Integer, Function<Integer, String>> evalUser = Map.of(PAPER, CHECK_PAPER, ROCK, CHECK_ROCK, SCISSORS, CHECK_SCISSORS); public static void main(String[] args) { new RockPaperScissors().play(); } public void play() { Random r = new Random(); Scanner scan = new Scanner(System.in); while (true) { System.out.printf("%n%d : %s%n%d : %s%n%d : %s%n%s%n", PAPER, "PAPER", ROCK, "ROCK", SCISSORS, "SCISSORS", "Any other integer to quit."); System.out.print("Your move! "); String str = scan.nextLine(); int move; try { move = Integer.parseInt(str); if (!allowedMoves.contains(move)) { break; } } catch (IllegalArgumentException ie) { System.out.println("Only integers permitted."); continue; } System.out.println("\nYou chose " + moves.get(move - 1)); int cmove = r.nextInt(3); System.out.println( "The computer chooses " + moves.get(cmove)); System.out.println(evalUser.get(move).apply(cmove + 1)); } System.out.println("\nGame over!"); } } 

Once suggestion for your code would be to look at the switch cases. The code for each case is practically identical. I would look for similarities and make the evaluation a single method (something I didn't really do in my code). Then in each case, call that method with the appropriate arguments. One such argument would be either "computer" or "you" based on the context.

There's nothing that ever sets playIntro false, and therefore the outer loop will never terminate.

When askUser() sets playGame false the inner loop terminates, and you fall into the outer loop, which keeps on looping.

I don't see any reason for the outer loop to exist at all. You only want to print the introduction and ask the player's name once.

This isn't so much a matter of 'efficiency' as of correctness.

Incidentally, it would be better to make askUser() return a true/false value rather than set a member variable. Then you can use it directly in a 'while' expression.

The overall structure of playRPS() then looks like:

public void playRPS() { ... print intro, ask name ... do { ... play one game ... } while (askUser()); }