-1

I just can't figure out why I'm getting a NullPointerException. The program gets to the first case and says i is not pointing to anything.

public String computerChoice()
{   
    int i = ran.nextInt(4);
    String str;
    switch (i){
    case 1:  
        str = "paper";
    case 2: 
        str = "rock";
    case 3: 
        str = "scissors";
    default:
        str = "not valid";   
    }
    return str;
} 
Mark Rotteveel
  • 100,966
  • 191
  • 140
  • 197
  • What line raises it ? Can you share a reproducible code : include `ran` instanciation/definition – azro Jan 02 '22 at 09:48
  • Assumption: `ran` is `null`. Solution: Change `int i = ran.nextInt(4);` to `int i = ThreadLocalRandom.current().nextInt(4);` it should probably be `int i = ThreadLocalRandom.current().nextInt(4) + 1;` – Elliott Frisch Jan 02 '22 at 09:50
  • You need to add the `break;` statement in each `case` block otherwise you get fall-through right to `default`. Also change the `int i = rnd.nextInt(4);` to `int i = rnd.nextInt(3) + 1;` as already suggested. Use `3` instead of `4` then you don't need to worry about default, or...is that a requirement in the game?. I like @Alex's array suggestion. – DevilsHnd - 退職した Jan 02 '22 at 10:47

1 Answers1

1

Therre are several issues in your code:

  1. It does not show that the Random instance ran is initialized and this should be the root cause of NullPointerException
  2. switch statement is written without break or return, therefore, "not valid" would be always returned after fixing the NPE
  3. Only three valid options should be generated and used in this rock-scissors-stone generator.

Possible fixes:

  1. Use Java 12 switch expression syntax:

public String computerChoice() {   
    return switch (new Random().nextInt(3)) {  // values in range [0..2]
        case 0 -> "paper";
        case 1 -> "rock";
        default -> "scissors"; // the only remaining value 2 should be default
    }
} 
  1. Use return in older switch statement:
public String computerChoice() {   
    switch (new Random().nextInt(3)) {  // values in range [0..2]
        case 0: return "paper";
        case 1: return "rock";
        default: return "scissors";
    }
} 
  1. Create and use an array of possible values without any switch at all to reduce the code complexity:
private static final String PRS = {"paper", "rock", "scissors"};

public String computerChoice() {   
    return PRS[new Random().nextInt(PRS.length)];
} 
Nowhere Man
  • 19,170
  • 9
  • 17
  • 42