Java Else If Block Not Working Correctly

Question

Hopefully this is a quick question. I'm making a simple rock paper scissors game. I make a random computer choice fine, and get the user's choice fine. But when I try to find who wins, it prints the last else in my else if block that is for invalid input.

It prints "Enter a valid choice" when a correct choice has been made.

import java.util.Random;
import javax.swing.JOptionPane;

public class JavaApplication4 {

    public static void main(String[] args) 
    {
        Random ranNums = new Random();

        int comp = ranNums.nextInt(3);
        String comp2;
        String winner;

        String user = JOptionPane.showInputDialog
                (null, "Enter rock, paper, or scissors");
        user.toLowerCase();

        if(comp == 0)
            comp2 = "rock";
        else if(comp == 1)
            comp2 = "paper";
        else 
            comp2 = "scissors";

        //Computer wins
        if(comp2 == "rock" && user == "scissors")
            winner = "The computer wins";

        else if(comp2 == "paper" && user == "rock")
            winner = "The computer wins";    

        else if(comp2 == "scissors" && user == "paper")
            winner = "The computer wins";

        //Tie game
        else if(comp2 == "rock" && user == "rock")
            winner = "It's a tie";

        else if(comp2 == "paper" && user == "paper")
            winner = "It's a tie";

        else if(comp2 == "scissors" && user == "scissors")
            winner = "It's a tie";

        //User wins
        else if(comp2 == "scissors" && user == "rock")
            winner = "You win!";

        else if(comp2 == "rock" && user == "paper")
            winner = "You win!";

        else if(comp2 == "paper" && user == "scissors")
            winner = "You win!";
        else
            winner = "Enter a valid choice";

        JOptionPane.showMessageDialog(null, "You picked " + user + "\n" +
            "The computer picked " + comp2 + "\n" +
                winner);



    }
}

I agree it's a duplicate. I didn't know it was a string comparing issue when I asked. — Philip Rego, Jan 25 '18 at 15:41

score 4 · Accepted Answer · answered Apr 06 '12 at 01:37

4

you can't use

comp == "paper"

to compare strings in java. You use

comp.equals("paper")

or if case doesn't matter

comp.equalsIgnoreCase("paper")

answered Apr 06 '12 at 01:37

twain249

5,666
1
21
26

3

To avoid NullPointerException it's better with "paper".equals(comp) – Uhlen Apr 06 '12 at 01:55

Hovercraft Full Of Eels · Answer 2 · 2012-04-06T01:52:16.833

Don't compare Strings using ==. Use the equals or the equalsIgnoreCase(...) method instead. Understand that == checks if the two objects are the same which is not what you're interested in. The methods on the other hand check if the two Strings have the same characters in the same order, and that's what matters here. So instead of

if (fu == "bar") {
  // do something
}

do,

if ("bar".equals(fu)) {
  // do something
}

or,

if ("bar".equalsIgnoreCase(fu)) {
  // do something
}

One way to simplify your code is to create a RockPaperScissors enum and give it its on compare method. Something like:

enum RockPaperScissors {
   ROCK("Rock"), PAPER("Paper"), SCISSORS("Scissors");

   private String text;
   private static int[][] winMatrix = {{0, -1, 1}, {1, 0, -1}, {-1, 1, 0}};

   private RockPaperScissors(String text) {
      this.text = text;
   }

   @Override
   public String toString() {
      return text;
   }

   // can't use compareTo since it is a final method for enums
   public int compareVs(RockPaperScissors other) {
      int thisOrdinal = ordinal();
      int otherOrdinal = other.ordinal();
      return winMatrix[thisOrdinal][otherOrdinal];
   }
}

Then to compare one enum vs. another, simply call its compareVs(...) method passing in the other enum.

So your huge if/else block would reduce down to:

// assuming that user and comp are RockPaperScissors variables
int result = user.compareVs(comp);
if (result == 1) {
   System.out.println("You've won!");
} else if (result == 0) {
   System.out.println("It's a tie!");
} if (result == -1) {
   System.out.println("You've lost!");
}

To avoid NullPointerException it's better with "bar".equals(fu) — Uhlen, Apr 06 '12 at 01:52
@Uhlen: thank you for the excellent suggestion! Code changed. — Hovercraft Full Of Eels, Apr 06 '12 at 01:54
Thank you for showing me how to optimize the code! I'm not very familiar with enums, but when I read about them I'll definitely check back here. — Philip Rego, Apr 06 '12 at 02:04

score 2 · Answer 3 · answered Apr 06 '12 at 01:43

If you are like me and you enjoy raw code:

import java.util.Random; 
import javax.swing.JOptionPane;

public class JavaApplication4 {

public static void main(String[] args) 
{
    Random ranNums = new Random();

    int comp = ranNums.nextInt(3);
    String comp2;
    String winner;

    String user = JOptionPane.showInputDialog
            (null, "Enter rock, paper, or scissors");
    user.toLowerCase();

    if(comp == 0)
        comp2 = "rock";
    else if(comp == 1)
        comp2 = "paper";
    else 
        comp2 = "scissors";

    //Computer wins
    if(comp2.equals("rock") && user.equals( "scissors"))
        winner = "The computer wins";

    else if(comp2.equals("paper") && user.equals( "rock"))
        winner = "The computer wins";    

    else if(comp2.equals("scissors") && user.equals( "paper"))
        winner = "The computer wins";

    //Tie game
    else if(comp2.equals("rock") && user.equals( "rock"))
        winner = "It's a tie";

    else if(comp2.equals("paper") && user.equals( "paper"))
        winner = "It's a tie";

    else if(comp2.equals("scissors") && user.equals( "scissors"))
        winner = "It's a tie";

    //User wins
    else if(comp2.equals("scissors") && user.equals( "rock"))
        winner = "You win!";

    else if(comp2.equals("rock") && user.equals( "paper"))
        winner = "You win!";

    else if(comp2.equals("paper") && user.equals( "scissors"))
        winner = "You win!";
    else
        winner = "Enter a valid choice";

    JOptionPane.showMessageDialog(null, "You picked " + user + "\n" +
        "The computer picked " + comp2 + "\n" +
            winner);
}
}

Hope that helped!

To avoid NullPointerException it's better with "rock".equals(comp2) — Uhlen, Apr 06 '12 at 01:54
@ Uhlen I know, but I did this with Eclipse's Replace feature, and that would have been a bit hard with regex. — Xyene, Apr 06 '12 at 02:02

score 1 · Answer 4 · answered Apr 06 '12 at 01:36

1

You cannot compare string like this: if (comp2 == "rock")

You need to write: if ("rock".equals(comp2))

answered Apr 06 '12 at 01:36

Eugene Retunsky

13,009
4
52
55

Java Else If Block Not Working Correctly

4 Answers4