Beginner Rock, Paper, Scissors in Java











up vote
5
down vote

favorite












I was curious on what, if anything, I could be doing better. I have been spending the last few days working with Java and learning some basics and wanted to try this challenge and making it two classes. I found the scanner is sometimes strange and doesn't take input the first time.



import java.util.Scanner;
import java.util.Random;

public class rps {
public static void main (String args){
int input;
int b = 1;
Scanner sage = new Scanner(System.in);
Random rnd = new Random();
System.out.println("Rock Paper Scissors, by Sage!");
System.out.println("Select 1, 2, 3, for Rock, Paper, Scissors");
//Menu Present, pretty bad still

while (b != 0){
int rock = 1, paper = 2, scissors = 3;
input = sage.nextInt();
int randomNumber = rnd.nextInt(3-1+1)+1;

if(randomNumber == rock){
if(input == rock){
System.out.println("Rock vs. Rock, ITS A TIE!");
} else if(input == paper){
System.out.println("Rock vs. Paper! You win!" );
} else if(input == scissors){
System.out.println("Rock vs. Scissors! You lose!");
} //These blocks establish options if the computer got Rock

else if(randomNumber == paper){
if(input == rock){
System.out.println("Paper vs. Rock! You lose!");
} else if(input == scissors){
System.out.println("Paper vs. Scissors! You win!");
} else if(input == paper){
System.out.println("Paper vs. Paper! Its a tie!");
} //These blocks establish the options if comp. got paper

else if(randomNumber == scissors){
if(input == rock){
System.out.println("Scissors vs. Rock! You win!");
} else if(input == scissors){
System.out.println("Scissors vs. Scissors, ITS A TIE!");
} else if(input == paper){
System.out.println("Scissors vs Paper! You lose!");
} //These blocks establish if the computer got scissors.

}
}
rps2 rps2Object = new rps2();
rps2Object.rps2();


}

}
}
}


Then this is the other class



import java.util.Scanner;

public class rps2 {
public void rps2(){
Scanner sage = new Scanner(System.in);
int b;
b = 1;
System.out.println("Play again? Y(8), N(9)?");
int yes= 8, no = 9;
int input;
input = sage.nextInt();

if(input == yes){
System.out.println("Rock,Paper,Scissors!");

}else{
System.out.println("Thanks for playing!");
}



}
}









share|improve this question




























    up vote
    5
    down vote

    favorite












    I was curious on what, if anything, I could be doing better. I have been spending the last few days working with Java and learning some basics and wanted to try this challenge and making it two classes. I found the scanner is sometimes strange and doesn't take input the first time.



    import java.util.Scanner;
    import java.util.Random;

    public class rps {
    public static void main (String args){
    int input;
    int b = 1;
    Scanner sage = new Scanner(System.in);
    Random rnd = new Random();
    System.out.println("Rock Paper Scissors, by Sage!");
    System.out.println("Select 1, 2, 3, for Rock, Paper, Scissors");
    //Menu Present, pretty bad still

    while (b != 0){
    int rock = 1, paper = 2, scissors = 3;
    input = sage.nextInt();
    int randomNumber = rnd.nextInt(3-1+1)+1;

    if(randomNumber == rock){
    if(input == rock){
    System.out.println("Rock vs. Rock, ITS A TIE!");
    } else if(input == paper){
    System.out.println("Rock vs. Paper! You win!" );
    } else if(input == scissors){
    System.out.println("Rock vs. Scissors! You lose!");
    } //These blocks establish options if the computer got Rock

    else if(randomNumber == paper){
    if(input == rock){
    System.out.println("Paper vs. Rock! You lose!");
    } else if(input == scissors){
    System.out.println("Paper vs. Scissors! You win!");
    } else if(input == paper){
    System.out.println("Paper vs. Paper! Its a tie!");
    } //These blocks establish the options if comp. got paper

    else if(randomNumber == scissors){
    if(input == rock){
    System.out.println("Scissors vs. Rock! You win!");
    } else if(input == scissors){
    System.out.println("Scissors vs. Scissors, ITS A TIE!");
    } else if(input == paper){
    System.out.println("Scissors vs Paper! You lose!");
    } //These blocks establish if the computer got scissors.

    }
    }
    rps2 rps2Object = new rps2();
    rps2Object.rps2();


    }

    }
    }
    }


    Then this is the other class



    import java.util.Scanner;

    public class rps2 {
    public void rps2(){
    Scanner sage = new Scanner(System.in);
    int b;
    b = 1;
    System.out.println("Play again? Y(8), N(9)?");
    int yes= 8, no = 9;
    int input;
    input = sage.nextInt();

    if(input == yes){
    System.out.println("Rock,Paper,Scissors!");

    }else{
    System.out.println("Thanks for playing!");
    }



    }
    }









    share|improve this question


























      up vote
      5
      down vote

      favorite









      up vote
      5
      down vote

      favorite











      I was curious on what, if anything, I could be doing better. I have been spending the last few days working with Java and learning some basics and wanted to try this challenge and making it two classes. I found the scanner is sometimes strange and doesn't take input the first time.



      import java.util.Scanner;
      import java.util.Random;

      public class rps {
      public static void main (String args){
      int input;
      int b = 1;
      Scanner sage = new Scanner(System.in);
      Random rnd = new Random();
      System.out.println("Rock Paper Scissors, by Sage!");
      System.out.println("Select 1, 2, 3, for Rock, Paper, Scissors");
      //Menu Present, pretty bad still

      while (b != 0){
      int rock = 1, paper = 2, scissors = 3;
      input = sage.nextInt();
      int randomNumber = rnd.nextInt(3-1+1)+1;

      if(randomNumber == rock){
      if(input == rock){
      System.out.println("Rock vs. Rock, ITS A TIE!");
      } else if(input == paper){
      System.out.println("Rock vs. Paper! You win!" );
      } else if(input == scissors){
      System.out.println("Rock vs. Scissors! You lose!");
      } //These blocks establish options if the computer got Rock

      else if(randomNumber == paper){
      if(input == rock){
      System.out.println("Paper vs. Rock! You lose!");
      } else if(input == scissors){
      System.out.println("Paper vs. Scissors! You win!");
      } else if(input == paper){
      System.out.println("Paper vs. Paper! Its a tie!");
      } //These blocks establish the options if comp. got paper

      else if(randomNumber == scissors){
      if(input == rock){
      System.out.println("Scissors vs. Rock! You win!");
      } else if(input == scissors){
      System.out.println("Scissors vs. Scissors, ITS A TIE!");
      } else if(input == paper){
      System.out.println("Scissors vs Paper! You lose!");
      } //These blocks establish if the computer got scissors.

      }
      }
      rps2 rps2Object = new rps2();
      rps2Object.rps2();


      }

      }
      }
      }


      Then this is the other class



      import java.util.Scanner;

      public class rps2 {
      public void rps2(){
      Scanner sage = new Scanner(System.in);
      int b;
      b = 1;
      System.out.println("Play again? Y(8), N(9)?");
      int yes= 8, no = 9;
      int input;
      input = sage.nextInt();

      if(input == yes){
      System.out.println("Rock,Paper,Scissors!");

      }else{
      System.out.println("Thanks for playing!");
      }



      }
      }









      share|improve this question















      I was curious on what, if anything, I could be doing better. I have been spending the last few days working with Java and learning some basics and wanted to try this challenge and making it two classes. I found the scanner is sometimes strange and doesn't take input the first time.



      import java.util.Scanner;
      import java.util.Random;

      public class rps {
      public static void main (String args){
      int input;
      int b = 1;
      Scanner sage = new Scanner(System.in);
      Random rnd = new Random();
      System.out.println("Rock Paper Scissors, by Sage!");
      System.out.println("Select 1, 2, 3, for Rock, Paper, Scissors");
      //Menu Present, pretty bad still

      while (b != 0){
      int rock = 1, paper = 2, scissors = 3;
      input = sage.nextInt();
      int randomNumber = rnd.nextInt(3-1+1)+1;

      if(randomNumber == rock){
      if(input == rock){
      System.out.println("Rock vs. Rock, ITS A TIE!");
      } else if(input == paper){
      System.out.println("Rock vs. Paper! You win!" );
      } else if(input == scissors){
      System.out.println("Rock vs. Scissors! You lose!");
      } //These blocks establish options if the computer got Rock

      else if(randomNumber == paper){
      if(input == rock){
      System.out.println("Paper vs. Rock! You lose!");
      } else if(input == scissors){
      System.out.println("Paper vs. Scissors! You win!");
      } else if(input == paper){
      System.out.println("Paper vs. Paper! Its a tie!");
      } //These blocks establish the options if comp. got paper

      else if(randomNumber == scissors){
      if(input == rock){
      System.out.println("Scissors vs. Rock! You win!");
      } else if(input == scissors){
      System.out.println("Scissors vs. Scissors, ITS A TIE!");
      } else if(input == paper){
      System.out.println("Scissors vs Paper! You lose!");
      } //These blocks establish if the computer got scissors.

      }
      }
      rps2 rps2Object = new rps2();
      rps2Object.rps2();


      }

      }
      }
      }


      Then this is the other class



      import java.util.Scanner;

      public class rps2 {
      public void rps2(){
      Scanner sage = new Scanner(System.in);
      int b;
      b = 1;
      System.out.println("Play again? Y(8), N(9)?");
      int yes= 8, no = 9;
      int input;
      input = sage.nextInt();

      if(input == yes){
      System.out.println("Rock,Paper,Scissors!");

      }else{
      System.out.println("Thanks for playing!");
      }



      }
      }






      java beginner rock-paper-scissors






      share|improve this question















      share|improve this question













      share|improve this question




      share|improve this question








      edited Jul 5 '16 at 4:06









      Jamal

      30.2k11115226




      30.2k11115226










      asked Jul 5 '16 at 2:29









      PtSage

      26112




      26112






















          4 Answers
          4






          active

          oldest

          votes

















          up vote
          4
          down vote













          There are plenty of rock-paper-scissors implementations in java on this site. I'll try to keep my suggestions at a beginner level, though.



          First, I should point out that your "play again?" mechanism doesn't actually work: you ask the question, but never do anything with the result. Putting the "play again?" prompt in its own class doesn't make much sense; it can just be a function. That function should return a boolean result — true to play again, false to quit. In main(), the while (b != 0) loop would best be written as a do-while loop, since you want to run the game at least once without asking. b is a poor variable name; playAgain would be much more helpful.



          The main lesson that I think you should learn from this exercise is object-oriented thinking. Instead of input and randomNumber, how about thinking about a HumanPlayer and a RandomComputerPlayer, both of which can return a choice of "Rock", "Paper", or "Scissors" when asked to play()? The code would better model the game.



          To analyze the outcome, you have enumerated all 9 combinations. Of those, three outcomes are ties, which can easily be detected. It would also be better to group all of the winning outcomes and all of the losing outcomes together.



          RockPaperScissors.java



          import java.util.Random;
          import java.util.Scanner;

          public class RockPaperScissors {
          private static boolean playAgain(Scanner scanner) {
          System.out.println("Play again? Y(8), N(9)?");
          switch (scanner.nextInt()) {
          case 8:
          System.out.println("Rock, Paper, Scissors!");
          return true;
          default:
          System.out.println("Thanks for playing!");
          return false;
          }
          }

          public static void main(String args) {
          Scanner scanner = new Scanner(System.in);
          RPSPlayer computer = new RandomComputerPlayer(new Random());
          RPSPlayer human = new HumanPlayer(scanner);

          System.out.println("Rock Paper Scissors, by 200_success!");
          do {
          String comp = computer.play();
          String you = human.play();

          System.out.printf("%s vs. %s", comp, you);
          if (you.equals(comp)) {
          System.out.println(", IT'S A TIE!");
          } else if ( ("Rock".equals(you) && "Scissors".equals(comp)) ||
          ("Scissors".equals(you) && "Paper".equals(comp)) ||
          ("Paper".equals(you) && "Rock".equals(comp)) ) {
          System.out.println("! You win!");
          } else {
          assert (("Rock".equals(comp) && "Scissors".equals(you)) ||
          ("Scissors".equals(comp) && "Paper".equals(you)) ||
          ("Paper".equals(comp) && "Rock".equals(you)));
          System.out.println("! You lose!");
          }
          } while (playAgain(scanner));
          }
          }


          RPSPlayer.java



          public interface RPSPlayer {
          String CHOICES = new String { "Rock", "Paper", "Scissors" };

          /**
          * Returns one of "Rock", "Paper", or "Scissors".
          */
          String play();
          }


          HumanPlayer.java



          import java.util.Scanner;

          public class HumanPlayer implements RPSPlayer {
          private final Scanner scanner;

          public HumanPlayer(Scanner scanner) {
          this.scanner = scanner;
          }

          public String play() {
          System.out.println("Select 1, 2, or 3 for Rock, Paper, Scissors");
          int choice = this.scanner.nextInt();
          // Keeping things simple, not doing any validation here
          return CHOICES[choice - 1];
          }
          }


          RandomComputerPlayer.java



          import java.util.Random;

          public class RandomComputerPlayer implements RPSPlayer {
          private final Random random;

          public RandomComputerPlayer(Random random) {
          this.random = random;
          }

          public String play() {
          return CHOICES[this.random.nextInt(CHOICES.length)];
          }
          }





          share|improve this answer




























            up vote
            3
            down vote













            Code organization



            You created two classes, but didn't explain why. Don't create a class unless you have a good reason to do so. Example valid reasons to create a class:




            • model real world objects

            • model abstract objects

            • reduce complexity

            • hide implementation details


            I strongly recommend to read the book Code Complete (2nd edition). If you're impatient, you can jump directly to chapter 6, working classes.



            I suggest to combine the two classes. In addition, try to split the functionality to smaller functions, each with a single responsibility, and a descriptive name. The main method should do almost nothing, just call other functions that do the real work, which will be explained by their names. Chapter 7 in Code Complete can help you with this point.



            Variable declarations



            It's best to declare variables right before you use them. Don't declare variables at the top of a function of they are not used until the middle. Especially, don't declare a variable at a broader scope, when it can be declared in a more limited scope. In particular, the input variable should be declared inside the while loop.



            Naming



            It's impossible to guess what's going on here without reading the implementation of the rps2 class:




                   rps2 rps2Object = new rps2();
            rps2Object.rps2();



            Also note that the convention in Java is to use PascalCase for class names.



            These are also very poor names:




               int b = 1;
            Scanner sage = new Scanner(System.in);



            It's never a good idea to name program elements after your own name. scanner would be more appropriate, for example.



            It seems the value of b never changes, your using it to write an infinite loop. The idiomatic writing style of an infinite loop is this:



            while (true) {
            // ...
            }


            Usability




            "Play again? Y(8), N(9)?"



            That is, enter 8 to mean yes, and enter 9 to mean no? It would be more natural to use y to mean yes and n to mean no.






            share|improve this answer




























              up vote
              1
              down vote













              I decided to write my own little Rock Paper Scissors game just so you could see the same project from another fellow programmer's perspective.



              import java.util.Arrays;
              import java.util.Collections;
              import java.util.List;
              import java.util.Scanner;

              /**
              * Created on 7/4/2016.
              *
              */

              public class RockPaperScissors {
              public static void main(String args){
              List<Integer> computerMoves = Arrays.asList(1, 2, 3); // Used to generate random number.
              Scanner input = new Scanner(System.in);

              // Infinite loop until broken by exit case.
              loop : while(true){
              System.out.print("Rock, Paper, Scissors, Exit >>> ");
              String move = input.nextLine().toLowerCase();
              Collections.shuffle(computerMoves); // Shuffles list to achieve random numbers.

              switch(move){
              case "rock" : determineWinner(computerMoves.get(0), "Tie", "You Lose", "You Win"); break; // if move is rock
              case "paper" : determineWinner(computerMoves.get(0), "You Win", "Tie", "You Lose"); break; // if move is paper
              case "scissors" : determineWinner(computerMoves.get(0), "You Lose", "You Win", "Tie"); break; // if move is scissors
              case "exit" : break loop; // if move is exit
              case "" : continue loop; // if move is blank
              default : System.out.println("Invalid Input"); // if move is none of the above
              }
              }
              }

              // Used in the interest of code re-usability
              private static void determineWinner(int computerMove, String m1, String m2, String m3){
              if(computerMove == 1){ // computer move is rock
              System.out.printf("Computer Chose Rock, %s%n", m1);
              } else if(computerMove == 2){ // computer move is paper
              System.out.printf("Computer Chose Paper, %s%n", m2);
              } else { // computer move is scissors
              System.out.printf("Computer Chose Scissors, %s%n", m3);
              }
              }
              }


              Now, whether my coding decisions were the absolute best for this situation or not, there are still a lot of things you can take from my code. I tried to reuse some of the same tools that you used so you wouldn't be too lost but I also tried to implement some new things you might have never used or seen before.



              Formatting:



              Take note of the curly braces in my code, they emphasize the actual "blocks" of code. Your code is very hard to read because of poorly placed braces. Here is a link to another review, the top answer goes a lot more in depth on curly braces placement and has some decent visuals as well. Also while on the topic of indentation you have a few silly indentations through out.



                  Random rnd = new Random();
              System.out.println("Rock Paper Scissors, by Sage!");
              System.out.println("Select 1, 2, 3, for Rock, Paper, Scissors");
              //Menu Present, pretty bad still


              And



              input = sage.nextInt();

              if(input == yes){
              System.out.println("Rock,Paper,Scissors!");

              }else{
              System.out.println("Thanks for playing!");
              }


              Indentations are generally only followed by open curly braces. Not sure if these were just copy paste issues but worth mentioning.



              Functionality:



              This line stands out the most I believe,



              input = sage.nextInt();


              It accepts the next Integer, if given anything other than an integer your application will crash. Most of the time its a good idea to have some sort of check in there and correct a user or at least stop them from crashing the program. I deal with this by accepting a string and checking all expected input with a switch case statement. If a user gives this application something other than the 5 expected inputs, it will correct them by saying "Invalid Input".



              Another notable problem with your game is that it will never stop. int b in your rps class never changes. When asking the player if they want to continue, you will have to change int b so that it equals 0 for the game to stop. Because int b is in another class from rps2, you would reference it like so:



              rps.b = 0;


              Conclusion



              These are just a few important things to keep in mind. I'm sure someone will touch on some of the others.






              share|improve this answer























              • Appreciate it a ton! Format is definitely something I need to work on as it is horribly messy but the indentation rule of open curly braces will help more than you could imagine for me. I haven't even began to touch on strings yet. A lot of stuff in your program has been stuff I have not touched on but do understand to some degree, I love seeing it from another perspective. Once again, thank you for the input =).
                – PtSage
                Jul 5 '16 at 4:34










              • I edited a few more comments in there to try and clear some things up. Happy coding !
                – JSextonn
                Jul 5 '16 at 4:41


















              up vote
              1
              down vote













              General




              1. Represent expressive game elements in extra classes (Players, Rock/Scissor/Paper, Result, Situation, ...) for OOP

              2. Try to reduce case handling, use proper let structures work for you

              3. Follow JAVA conventions in Naming


              Code



              I looked after a solution with less if statements and more expressive artifacts. I came up with this:



              Introduce a class "Situation"



              This represents a game situation with the choice of each player:



              public class Situation {


              private final String choice1;
              private final String choice2;


              public Situation(String choice1, String choice2) {
              this.choice1 = choice1;
              this.choice2 = choice2;
              }


              @Override
              public int hashCode() {
              return choice1.hashCode() * choice2.hashCode();
              }


              @Override
              public boolean equals(Object obj) {

              boolean equals = false;

              if (obj instanceof Situation) {

              Situation that = (Situation) obj;

              equals = this.choice1.equals(that.choice1)
              && this.choice2.equals(that.choice2);

              }

              return equals;
              }


              public Situation invert() {
              return new Situation(choice2, choice1);
              }


              }


              It's a value object so recognize the immutability and hashcode/equals overriden.



              One other thing is the invert method which is a convenience method to express the other side around.



              You may consider to use "real" objects to represent "Rock", Scissor" and "Paper" instead of String-Objects.



              Introduce class "Result"



              It represents the outcome of one round.



              public enum Result {

              TIE, PLAYER1_WINS, PLAYER2_WINS

              }


              Evaluate all possible situations



              You can now easily provide a Set of possible winning situations to evaluate the winner.



              public class RockScissorPaper {


              private Set<Situation> winSituations;


              public RockScissorPaper() {

              this.winSituations = new HashSet<>();

              this.winSituations.add(new Situation("Rock", "Scissor"));
              this.winSituations.add(new Situation("Scissor", "Paper"));
              this.winSituations.add(new Situation("Paper", "Rock"));

              }


              public Result evaluateWinner(Situation situation) {

              if (this.winSituations.contains(situation)) {
              return Result.PLAYER1_WINS;
              } else if (this.winSituations.contains(situation.invert())) {
              return Result.PLAYER2_WINS;
              } else {
              return Result.TIE;
              }

              }


              public static void main(String args) {

              RockScissorPaper rockScissorPaper = new RockScissorPaper();

              System.out.println(rockScissorPaper.evaluateWinner(new Situation("Rock", "Rock"))); // --> TIE
              System.out.println(rockScissorPaper.evaluateWinner(new Situation("Scissor", "Paper"))); // --> PLAYER1_WINS
              System.out.println(rockScissorPaper.evaluateWinner(new Situation("Rock", "Paper"))); // --> PLAYER2_WINS

              }


              }





              share|improve this answer




















                protected by Jamal 33 mins ago



                Thank you for your interest in this question.
                Because it has attracted low-quality or spam answers that had to be removed, posting an answer now requires 10 reputation on this site (the association bonus does not count).



                Would you like to answer one of these unanswered questions instead?














                4 Answers
                4






                active

                oldest

                votes








                4 Answers
                4






                active

                oldest

                votes









                active

                oldest

                votes






                active

                oldest

                votes








                up vote
                4
                down vote













                There are plenty of rock-paper-scissors implementations in java on this site. I'll try to keep my suggestions at a beginner level, though.



                First, I should point out that your "play again?" mechanism doesn't actually work: you ask the question, but never do anything with the result. Putting the "play again?" prompt in its own class doesn't make much sense; it can just be a function. That function should return a boolean result — true to play again, false to quit. In main(), the while (b != 0) loop would best be written as a do-while loop, since you want to run the game at least once without asking. b is a poor variable name; playAgain would be much more helpful.



                The main lesson that I think you should learn from this exercise is object-oriented thinking. Instead of input and randomNumber, how about thinking about a HumanPlayer and a RandomComputerPlayer, both of which can return a choice of "Rock", "Paper", or "Scissors" when asked to play()? The code would better model the game.



                To analyze the outcome, you have enumerated all 9 combinations. Of those, three outcomes are ties, which can easily be detected. It would also be better to group all of the winning outcomes and all of the losing outcomes together.



                RockPaperScissors.java



                import java.util.Random;
                import java.util.Scanner;

                public class RockPaperScissors {
                private static boolean playAgain(Scanner scanner) {
                System.out.println("Play again? Y(8), N(9)?");
                switch (scanner.nextInt()) {
                case 8:
                System.out.println("Rock, Paper, Scissors!");
                return true;
                default:
                System.out.println("Thanks for playing!");
                return false;
                }
                }

                public static void main(String args) {
                Scanner scanner = new Scanner(System.in);
                RPSPlayer computer = new RandomComputerPlayer(new Random());
                RPSPlayer human = new HumanPlayer(scanner);

                System.out.println("Rock Paper Scissors, by 200_success!");
                do {
                String comp = computer.play();
                String you = human.play();

                System.out.printf("%s vs. %s", comp, you);
                if (you.equals(comp)) {
                System.out.println(", IT'S A TIE!");
                } else if ( ("Rock".equals(you) && "Scissors".equals(comp)) ||
                ("Scissors".equals(you) && "Paper".equals(comp)) ||
                ("Paper".equals(you) && "Rock".equals(comp)) ) {
                System.out.println("! You win!");
                } else {
                assert (("Rock".equals(comp) && "Scissors".equals(you)) ||
                ("Scissors".equals(comp) && "Paper".equals(you)) ||
                ("Paper".equals(comp) && "Rock".equals(you)));
                System.out.println("! You lose!");
                }
                } while (playAgain(scanner));
                }
                }


                RPSPlayer.java



                public interface RPSPlayer {
                String CHOICES = new String { "Rock", "Paper", "Scissors" };

                /**
                * Returns one of "Rock", "Paper", or "Scissors".
                */
                String play();
                }


                HumanPlayer.java



                import java.util.Scanner;

                public class HumanPlayer implements RPSPlayer {
                private final Scanner scanner;

                public HumanPlayer(Scanner scanner) {
                this.scanner = scanner;
                }

                public String play() {
                System.out.println("Select 1, 2, or 3 for Rock, Paper, Scissors");
                int choice = this.scanner.nextInt();
                // Keeping things simple, not doing any validation here
                return CHOICES[choice - 1];
                }
                }


                RandomComputerPlayer.java



                import java.util.Random;

                public class RandomComputerPlayer implements RPSPlayer {
                private final Random random;

                public RandomComputerPlayer(Random random) {
                this.random = random;
                }

                public String play() {
                return CHOICES[this.random.nextInt(CHOICES.length)];
                }
                }





                share|improve this answer

























                  up vote
                  4
                  down vote













                  There are plenty of rock-paper-scissors implementations in java on this site. I'll try to keep my suggestions at a beginner level, though.



                  First, I should point out that your "play again?" mechanism doesn't actually work: you ask the question, but never do anything with the result. Putting the "play again?" prompt in its own class doesn't make much sense; it can just be a function. That function should return a boolean result — true to play again, false to quit. In main(), the while (b != 0) loop would best be written as a do-while loop, since you want to run the game at least once without asking. b is a poor variable name; playAgain would be much more helpful.



                  The main lesson that I think you should learn from this exercise is object-oriented thinking. Instead of input and randomNumber, how about thinking about a HumanPlayer and a RandomComputerPlayer, both of which can return a choice of "Rock", "Paper", or "Scissors" when asked to play()? The code would better model the game.



                  To analyze the outcome, you have enumerated all 9 combinations. Of those, three outcomes are ties, which can easily be detected. It would also be better to group all of the winning outcomes and all of the losing outcomes together.



                  RockPaperScissors.java



                  import java.util.Random;
                  import java.util.Scanner;

                  public class RockPaperScissors {
                  private static boolean playAgain(Scanner scanner) {
                  System.out.println("Play again? Y(8), N(9)?");
                  switch (scanner.nextInt()) {
                  case 8:
                  System.out.println("Rock, Paper, Scissors!");
                  return true;
                  default:
                  System.out.println("Thanks for playing!");
                  return false;
                  }
                  }

                  public static void main(String args) {
                  Scanner scanner = new Scanner(System.in);
                  RPSPlayer computer = new RandomComputerPlayer(new Random());
                  RPSPlayer human = new HumanPlayer(scanner);

                  System.out.println("Rock Paper Scissors, by 200_success!");
                  do {
                  String comp = computer.play();
                  String you = human.play();

                  System.out.printf("%s vs. %s", comp, you);
                  if (you.equals(comp)) {
                  System.out.println(", IT'S A TIE!");
                  } else if ( ("Rock".equals(you) && "Scissors".equals(comp)) ||
                  ("Scissors".equals(you) && "Paper".equals(comp)) ||
                  ("Paper".equals(you) && "Rock".equals(comp)) ) {
                  System.out.println("! You win!");
                  } else {
                  assert (("Rock".equals(comp) && "Scissors".equals(you)) ||
                  ("Scissors".equals(comp) && "Paper".equals(you)) ||
                  ("Paper".equals(comp) && "Rock".equals(you)));
                  System.out.println("! You lose!");
                  }
                  } while (playAgain(scanner));
                  }
                  }


                  RPSPlayer.java



                  public interface RPSPlayer {
                  String CHOICES = new String { "Rock", "Paper", "Scissors" };

                  /**
                  * Returns one of "Rock", "Paper", or "Scissors".
                  */
                  String play();
                  }


                  HumanPlayer.java



                  import java.util.Scanner;

                  public class HumanPlayer implements RPSPlayer {
                  private final Scanner scanner;

                  public HumanPlayer(Scanner scanner) {
                  this.scanner = scanner;
                  }

                  public String play() {
                  System.out.println("Select 1, 2, or 3 for Rock, Paper, Scissors");
                  int choice = this.scanner.nextInt();
                  // Keeping things simple, not doing any validation here
                  return CHOICES[choice - 1];
                  }
                  }


                  RandomComputerPlayer.java



                  import java.util.Random;

                  public class RandomComputerPlayer implements RPSPlayer {
                  private final Random random;

                  public RandomComputerPlayer(Random random) {
                  this.random = random;
                  }

                  public String play() {
                  return CHOICES[this.random.nextInt(CHOICES.length)];
                  }
                  }





                  share|improve this answer























                    up vote
                    4
                    down vote










                    up vote
                    4
                    down vote









                    There are plenty of rock-paper-scissors implementations in java on this site. I'll try to keep my suggestions at a beginner level, though.



                    First, I should point out that your "play again?" mechanism doesn't actually work: you ask the question, but never do anything with the result. Putting the "play again?" prompt in its own class doesn't make much sense; it can just be a function. That function should return a boolean result — true to play again, false to quit. In main(), the while (b != 0) loop would best be written as a do-while loop, since you want to run the game at least once without asking. b is a poor variable name; playAgain would be much more helpful.



                    The main lesson that I think you should learn from this exercise is object-oriented thinking. Instead of input and randomNumber, how about thinking about a HumanPlayer and a RandomComputerPlayer, both of which can return a choice of "Rock", "Paper", or "Scissors" when asked to play()? The code would better model the game.



                    To analyze the outcome, you have enumerated all 9 combinations. Of those, three outcomes are ties, which can easily be detected. It would also be better to group all of the winning outcomes and all of the losing outcomes together.



                    RockPaperScissors.java



                    import java.util.Random;
                    import java.util.Scanner;

                    public class RockPaperScissors {
                    private static boolean playAgain(Scanner scanner) {
                    System.out.println("Play again? Y(8), N(9)?");
                    switch (scanner.nextInt()) {
                    case 8:
                    System.out.println("Rock, Paper, Scissors!");
                    return true;
                    default:
                    System.out.println("Thanks for playing!");
                    return false;
                    }
                    }

                    public static void main(String args) {
                    Scanner scanner = new Scanner(System.in);
                    RPSPlayer computer = new RandomComputerPlayer(new Random());
                    RPSPlayer human = new HumanPlayer(scanner);

                    System.out.println("Rock Paper Scissors, by 200_success!");
                    do {
                    String comp = computer.play();
                    String you = human.play();

                    System.out.printf("%s vs. %s", comp, you);
                    if (you.equals(comp)) {
                    System.out.println(", IT'S A TIE!");
                    } else if ( ("Rock".equals(you) && "Scissors".equals(comp)) ||
                    ("Scissors".equals(you) && "Paper".equals(comp)) ||
                    ("Paper".equals(you) && "Rock".equals(comp)) ) {
                    System.out.println("! You win!");
                    } else {
                    assert (("Rock".equals(comp) && "Scissors".equals(you)) ||
                    ("Scissors".equals(comp) && "Paper".equals(you)) ||
                    ("Paper".equals(comp) && "Rock".equals(you)));
                    System.out.println("! You lose!");
                    }
                    } while (playAgain(scanner));
                    }
                    }


                    RPSPlayer.java



                    public interface RPSPlayer {
                    String CHOICES = new String { "Rock", "Paper", "Scissors" };

                    /**
                    * Returns one of "Rock", "Paper", or "Scissors".
                    */
                    String play();
                    }


                    HumanPlayer.java



                    import java.util.Scanner;

                    public class HumanPlayer implements RPSPlayer {
                    private final Scanner scanner;

                    public HumanPlayer(Scanner scanner) {
                    this.scanner = scanner;
                    }

                    public String play() {
                    System.out.println("Select 1, 2, or 3 for Rock, Paper, Scissors");
                    int choice = this.scanner.nextInt();
                    // Keeping things simple, not doing any validation here
                    return CHOICES[choice - 1];
                    }
                    }


                    RandomComputerPlayer.java



                    import java.util.Random;

                    public class RandomComputerPlayer implements RPSPlayer {
                    private final Random random;

                    public RandomComputerPlayer(Random random) {
                    this.random = random;
                    }

                    public String play() {
                    return CHOICES[this.random.nextInt(CHOICES.length)];
                    }
                    }





                    share|improve this answer












                    There are plenty of rock-paper-scissors implementations in java on this site. I'll try to keep my suggestions at a beginner level, though.



                    First, I should point out that your "play again?" mechanism doesn't actually work: you ask the question, but never do anything with the result. Putting the "play again?" prompt in its own class doesn't make much sense; it can just be a function. That function should return a boolean result — true to play again, false to quit. In main(), the while (b != 0) loop would best be written as a do-while loop, since you want to run the game at least once without asking. b is a poor variable name; playAgain would be much more helpful.



                    The main lesson that I think you should learn from this exercise is object-oriented thinking. Instead of input and randomNumber, how about thinking about a HumanPlayer and a RandomComputerPlayer, both of which can return a choice of "Rock", "Paper", or "Scissors" when asked to play()? The code would better model the game.



                    To analyze the outcome, you have enumerated all 9 combinations. Of those, three outcomes are ties, which can easily be detected. It would also be better to group all of the winning outcomes and all of the losing outcomes together.



                    RockPaperScissors.java



                    import java.util.Random;
                    import java.util.Scanner;

                    public class RockPaperScissors {
                    private static boolean playAgain(Scanner scanner) {
                    System.out.println("Play again? Y(8), N(9)?");
                    switch (scanner.nextInt()) {
                    case 8:
                    System.out.println("Rock, Paper, Scissors!");
                    return true;
                    default:
                    System.out.println("Thanks for playing!");
                    return false;
                    }
                    }

                    public static void main(String args) {
                    Scanner scanner = new Scanner(System.in);
                    RPSPlayer computer = new RandomComputerPlayer(new Random());
                    RPSPlayer human = new HumanPlayer(scanner);

                    System.out.println("Rock Paper Scissors, by 200_success!");
                    do {
                    String comp = computer.play();
                    String you = human.play();

                    System.out.printf("%s vs. %s", comp, you);
                    if (you.equals(comp)) {
                    System.out.println(", IT'S A TIE!");
                    } else if ( ("Rock".equals(you) && "Scissors".equals(comp)) ||
                    ("Scissors".equals(you) && "Paper".equals(comp)) ||
                    ("Paper".equals(you) && "Rock".equals(comp)) ) {
                    System.out.println("! You win!");
                    } else {
                    assert (("Rock".equals(comp) && "Scissors".equals(you)) ||
                    ("Scissors".equals(comp) && "Paper".equals(you)) ||
                    ("Paper".equals(comp) && "Rock".equals(you)));
                    System.out.println("! You lose!");
                    }
                    } while (playAgain(scanner));
                    }
                    }


                    RPSPlayer.java



                    public interface RPSPlayer {
                    String CHOICES = new String { "Rock", "Paper", "Scissors" };

                    /**
                    * Returns one of "Rock", "Paper", or "Scissors".
                    */
                    String play();
                    }


                    HumanPlayer.java



                    import java.util.Scanner;

                    public class HumanPlayer implements RPSPlayer {
                    private final Scanner scanner;

                    public HumanPlayer(Scanner scanner) {
                    this.scanner = scanner;
                    }

                    public String play() {
                    System.out.println("Select 1, 2, or 3 for Rock, Paper, Scissors");
                    int choice = this.scanner.nextInt();
                    // Keeping things simple, not doing any validation here
                    return CHOICES[choice - 1];
                    }
                    }


                    RandomComputerPlayer.java



                    import java.util.Random;

                    public class RandomComputerPlayer implements RPSPlayer {
                    private final Random random;

                    public RandomComputerPlayer(Random random) {
                    this.random = random;
                    }

                    public String play() {
                    return CHOICES[this.random.nextInt(CHOICES.length)];
                    }
                    }






                    share|improve this answer












                    share|improve this answer



                    share|improve this answer










                    answered Jul 5 '16 at 5:44









                    200_success

                    128k15149412




                    128k15149412
























                        up vote
                        3
                        down vote













                        Code organization



                        You created two classes, but didn't explain why. Don't create a class unless you have a good reason to do so. Example valid reasons to create a class:




                        • model real world objects

                        • model abstract objects

                        • reduce complexity

                        • hide implementation details


                        I strongly recommend to read the book Code Complete (2nd edition). If you're impatient, you can jump directly to chapter 6, working classes.



                        I suggest to combine the two classes. In addition, try to split the functionality to smaller functions, each with a single responsibility, and a descriptive name. The main method should do almost nothing, just call other functions that do the real work, which will be explained by their names. Chapter 7 in Code Complete can help you with this point.



                        Variable declarations



                        It's best to declare variables right before you use them. Don't declare variables at the top of a function of they are not used until the middle. Especially, don't declare a variable at a broader scope, when it can be declared in a more limited scope. In particular, the input variable should be declared inside the while loop.



                        Naming



                        It's impossible to guess what's going on here without reading the implementation of the rps2 class:




                               rps2 rps2Object = new rps2();
                        rps2Object.rps2();



                        Also note that the convention in Java is to use PascalCase for class names.



                        These are also very poor names:




                           int b = 1;
                        Scanner sage = new Scanner(System.in);



                        It's never a good idea to name program elements after your own name. scanner would be more appropriate, for example.



                        It seems the value of b never changes, your using it to write an infinite loop. The idiomatic writing style of an infinite loop is this:



                        while (true) {
                        // ...
                        }


                        Usability




                        "Play again? Y(8), N(9)?"



                        That is, enter 8 to mean yes, and enter 9 to mean no? It would be more natural to use y to mean yes and n to mean no.






                        share|improve this answer

























                          up vote
                          3
                          down vote













                          Code organization



                          You created two classes, but didn't explain why. Don't create a class unless you have a good reason to do so. Example valid reasons to create a class:




                          • model real world objects

                          • model abstract objects

                          • reduce complexity

                          • hide implementation details


                          I strongly recommend to read the book Code Complete (2nd edition). If you're impatient, you can jump directly to chapter 6, working classes.



                          I suggest to combine the two classes. In addition, try to split the functionality to smaller functions, each with a single responsibility, and a descriptive name. The main method should do almost nothing, just call other functions that do the real work, which will be explained by their names. Chapter 7 in Code Complete can help you with this point.



                          Variable declarations



                          It's best to declare variables right before you use them. Don't declare variables at the top of a function of they are not used until the middle. Especially, don't declare a variable at a broader scope, when it can be declared in a more limited scope. In particular, the input variable should be declared inside the while loop.



                          Naming



                          It's impossible to guess what's going on here without reading the implementation of the rps2 class:




                                 rps2 rps2Object = new rps2();
                          rps2Object.rps2();



                          Also note that the convention in Java is to use PascalCase for class names.



                          These are also very poor names:




                             int b = 1;
                          Scanner sage = new Scanner(System.in);



                          It's never a good idea to name program elements after your own name. scanner would be more appropriate, for example.



                          It seems the value of b never changes, your using it to write an infinite loop. The idiomatic writing style of an infinite loop is this:



                          while (true) {
                          // ...
                          }


                          Usability




                          "Play again? Y(8), N(9)?"



                          That is, enter 8 to mean yes, and enter 9 to mean no? It would be more natural to use y to mean yes and n to mean no.






                          share|improve this answer























                            up vote
                            3
                            down vote










                            up vote
                            3
                            down vote









                            Code organization



                            You created two classes, but didn't explain why. Don't create a class unless you have a good reason to do so. Example valid reasons to create a class:




                            • model real world objects

                            • model abstract objects

                            • reduce complexity

                            • hide implementation details


                            I strongly recommend to read the book Code Complete (2nd edition). If you're impatient, you can jump directly to chapter 6, working classes.



                            I suggest to combine the two classes. In addition, try to split the functionality to smaller functions, each with a single responsibility, and a descriptive name. The main method should do almost nothing, just call other functions that do the real work, which will be explained by their names. Chapter 7 in Code Complete can help you with this point.



                            Variable declarations



                            It's best to declare variables right before you use them. Don't declare variables at the top of a function of they are not used until the middle. Especially, don't declare a variable at a broader scope, when it can be declared in a more limited scope. In particular, the input variable should be declared inside the while loop.



                            Naming



                            It's impossible to guess what's going on here without reading the implementation of the rps2 class:




                                   rps2 rps2Object = new rps2();
                            rps2Object.rps2();



                            Also note that the convention in Java is to use PascalCase for class names.



                            These are also very poor names:




                               int b = 1;
                            Scanner sage = new Scanner(System.in);



                            It's never a good idea to name program elements after your own name. scanner would be more appropriate, for example.



                            It seems the value of b never changes, your using it to write an infinite loop. The idiomatic writing style of an infinite loop is this:



                            while (true) {
                            // ...
                            }


                            Usability




                            "Play again? Y(8), N(9)?"



                            That is, enter 8 to mean yes, and enter 9 to mean no? It would be more natural to use y to mean yes and n to mean no.






                            share|improve this answer












                            Code organization



                            You created two classes, but didn't explain why. Don't create a class unless you have a good reason to do so. Example valid reasons to create a class:




                            • model real world objects

                            • model abstract objects

                            • reduce complexity

                            • hide implementation details


                            I strongly recommend to read the book Code Complete (2nd edition). If you're impatient, you can jump directly to chapter 6, working classes.



                            I suggest to combine the two classes. In addition, try to split the functionality to smaller functions, each with a single responsibility, and a descriptive name. The main method should do almost nothing, just call other functions that do the real work, which will be explained by their names. Chapter 7 in Code Complete can help you with this point.



                            Variable declarations



                            It's best to declare variables right before you use them. Don't declare variables at the top of a function of they are not used until the middle. Especially, don't declare a variable at a broader scope, when it can be declared in a more limited scope. In particular, the input variable should be declared inside the while loop.



                            Naming



                            It's impossible to guess what's going on here without reading the implementation of the rps2 class:




                                   rps2 rps2Object = new rps2();
                            rps2Object.rps2();



                            Also note that the convention in Java is to use PascalCase for class names.



                            These are also very poor names:




                               int b = 1;
                            Scanner sage = new Scanner(System.in);



                            It's never a good idea to name program elements after your own name. scanner would be more appropriate, for example.



                            It seems the value of b never changes, your using it to write an infinite loop. The idiomatic writing style of an infinite loop is this:



                            while (true) {
                            // ...
                            }


                            Usability




                            "Play again? Y(8), N(9)?"



                            That is, enter 8 to mean yes, and enter 9 to mean no? It would be more natural to use y to mean yes and n to mean no.







                            share|improve this answer












                            share|improve this answer



                            share|improve this answer










                            answered Jul 5 '16 at 6:03









                            janos

                            97k12124350




                            97k12124350






















                                up vote
                                1
                                down vote













                                I decided to write my own little Rock Paper Scissors game just so you could see the same project from another fellow programmer's perspective.



                                import java.util.Arrays;
                                import java.util.Collections;
                                import java.util.List;
                                import java.util.Scanner;

                                /**
                                * Created on 7/4/2016.
                                *
                                */

                                public class RockPaperScissors {
                                public static void main(String args){
                                List<Integer> computerMoves = Arrays.asList(1, 2, 3); // Used to generate random number.
                                Scanner input = new Scanner(System.in);

                                // Infinite loop until broken by exit case.
                                loop : while(true){
                                System.out.print("Rock, Paper, Scissors, Exit >>> ");
                                String move = input.nextLine().toLowerCase();
                                Collections.shuffle(computerMoves); // Shuffles list to achieve random numbers.

                                switch(move){
                                case "rock" : determineWinner(computerMoves.get(0), "Tie", "You Lose", "You Win"); break; // if move is rock
                                case "paper" : determineWinner(computerMoves.get(0), "You Win", "Tie", "You Lose"); break; // if move is paper
                                case "scissors" : determineWinner(computerMoves.get(0), "You Lose", "You Win", "Tie"); break; // if move is scissors
                                case "exit" : break loop; // if move is exit
                                case "" : continue loop; // if move is blank
                                default : System.out.println("Invalid Input"); // if move is none of the above
                                }
                                }
                                }

                                // Used in the interest of code re-usability
                                private static void determineWinner(int computerMove, String m1, String m2, String m3){
                                if(computerMove == 1){ // computer move is rock
                                System.out.printf("Computer Chose Rock, %s%n", m1);
                                } else if(computerMove == 2){ // computer move is paper
                                System.out.printf("Computer Chose Paper, %s%n", m2);
                                } else { // computer move is scissors
                                System.out.printf("Computer Chose Scissors, %s%n", m3);
                                }
                                }
                                }


                                Now, whether my coding decisions were the absolute best for this situation or not, there are still a lot of things you can take from my code. I tried to reuse some of the same tools that you used so you wouldn't be too lost but I also tried to implement some new things you might have never used or seen before.



                                Formatting:



                                Take note of the curly braces in my code, they emphasize the actual "blocks" of code. Your code is very hard to read because of poorly placed braces. Here is a link to another review, the top answer goes a lot more in depth on curly braces placement and has some decent visuals as well. Also while on the topic of indentation you have a few silly indentations through out.



                                    Random rnd = new Random();
                                System.out.println("Rock Paper Scissors, by Sage!");
                                System.out.println("Select 1, 2, 3, for Rock, Paper, Scissors");
                                //Menu Present, pretty bad still


                                And



                                input = sage.nextInt();

                                if(input == yes){
                                System.out.println("Rock,Paper,Scissors!");

                                }else{
                                System.out.println("Thanks for playing!");
                                }


                                Indentations are generally only followed by open curly braces. Not sure if these were just copy paste issues but worth mentioning.



                                Functionality:



                                This line stands out the most I believe,



                                input = sage.nextInt();


                                It accepts the next Integer, if given anything other than an integer your application will crash. Most of the time its a good idea to have some sort of check in there and correct a user or at least stop them from crashing the program. I deal with this by accepting a string and checking all expected input with a switch case statement. If a user gives this application something other than the 5 expected inputs, it will correct them by saying "Invalid Input".



                                Another notable problem with your game is that it will never stop. int b in your rps class never changes. When asking the player if they want to continue, you will have to change int b so that it equals 0 for the game to stop. Because int b is in another class from rps2, you would reference it like so:



                                rps.b = 0;


                                Conclusion



                                These are just a few important things to keep in mind. I'm sure someone will touch on some of the others.






                                share|improve this answer























                                • Appreciate it a ton! Format is definitely something I need to work on as it is horribly messy but the indentation rule of open curly braces will help more than you could imagine for me. I haven't even began to touch on strings yet. A lot of stuff in your program has been stuff I have not touched on but do understand to some degree, I love seeing it from another perspective. Once again, thank you for the input =).
                                  – PtSage
                                  Jul 5 '16 at 4:34










                                • I edited a few more comments in there to try and clear some things up. Happy coding !
                                  – JSextonn
                                  Jul 5 '16 at 4:41















                                up vote
                                1
                                down vote













                                I decided to write my own little Rock Paper Scissors game just so you could see the same project from another fellow programmer's perspective.



                                import java.util.Arrays;
                                import java.util.Collections;
                                import java.util.List;
                                import java.util.Scanner;

                                /**
                                * Created on 7/4/2016.
                                *
                                */

                                public class RockPaperScissors {
                                public static void main(String args){
                                List<Integer> computerMoves = Arrays.asList(1, 2, 3); // Used to generate random number.
                                Scanner input = new Scanner(System.in);

                                // Infinite loop until broken by exit case.
                                loop : while(true){
                                System.out.print("Rock, Paper, Scissors, Exit >>> ");
                                String move = input.nextLine().toLowerCase();
                                Collections.shuffle(computerMoves); // Shuffles list to achieve random numbers.

                                switch(move){
                                case "rock" : determineWinner(computerMoves.get(0), "Tie", "You Lose", "You Win"); break; // if move is rock
                                case "paper" : determineWinner(computerMoves.get(0), "You Win", "Tie", "You Lose"); break; // if move is paper
                                case "scissors" : determineWinner(computerMoves.get(0), "You Lose", "You Win", "Tie"); break; // if move is scissors
                                case "exit" : break loop; // if move is exit
                                case "" : continue loop; // if move is blank
                                default : System.out.println("Invalid Input"); // if move is none of the above
                                }
                                }
                                }

                                // Used in the interest of code re-usability
                                private static void determineWinner(int computerMove, String m1, String m2, String m3){
                                if(computerMove == 1){ // computer move is rock
                                System.out.printf("Computer Chose Rock, %s%n", m1);
                                } else if(computerMove == 2){ // computer move is paper
                                System.out.printf("Computer Chose Paper, %s%n", m2);
                                } else { // computer move is scissors
                                System.out.printf("Computer Chose Scissors, %s%n", m3);
                                }
                                }
                                }


                                Now, whether my coding decisions were the absolute best for this situation or not, there are still a lot of things you can take from my code. I tried to reuse some of the same tools that you used so you wouldn't be too lost but I also tried to implement some new things you might have never used or seen before.



                                Formatting:



                                Take note of the curly braces in my code, they emphasize the actual "blocks" of code. Your code is very hard to read because of poorly placed braces. Here is a link to another review, the top answer goes a lot more in depth on curly braces placement and has some decent visuals as well. Also while on the topic of indentation you have a few silly indentations through out.



                                    Random rnd = new Random();
                                System.out.println("Rock Paper Scissors, by Sage!");
                                System.out.println("Select 1, 2, 3, for Rock, Paper, Scissors");
                                //Menu Present, pretty bad still


                                And



                                input = sage.nextInt();

                                if(input == yes){
                                System.out.println("Rock,Paper,Scissors!");

                                }else{
                                System.out.println("Thanks for playing!");
                                }


                                Indentations are generally only followed by open curly braces. Not sure if these were just copy paste issues but worth mentioning.



                                Functionality:



                                This line stands out the most I believe,



                                input = sage.nextInt();


                                It accepts the next Integer, if given anything other than an integer your application will crash. Most of the time its a good idea to have some sort of check in there and correct a user or at least stop them from crashing the program. I deal with this by accepting a string and checking all expected input with a switch case statement. If a user gives this application something other than the 5 expected inputs, it will correct them by saying "Invalid Input".



                                Another notable problem with your game is that it will never stop. int b in your rps class never changes. When asking the player if they want to continue, you will have to change int b so that it equals 0 for the game to stop. Because int b is in another class from rps2, you would reference it like so:



                                rps.b = 0;


                                Conclusion



                                These are just a few important things to keep in mind. I'm sure someone will touch on some of the others.






                                share|improve this answer























                                • Appreciate it a ton! Format is definitely something I need to work on as it is horribly messy but the indentation rule of open curly braces will help more than you could imagine for me. I haven't even began to touch on strings yet. A lot of stuff in your program has been stuff I have not touched on but do understand to some degree, I love seeing it from another perspective. Once again, thank you for the input =).
                                  – PtSage
                                  Jul 5 '16 at 4:34










                                • I edited a few more comments in there to try and clear some things up. Happy coding !
                                  – JSextonn
                                  Jul 5 '16 at 4:41













                                up vote
                                1
                                down vote










                                up vote
                                1
                                down vote









                                I decided to write my own little Rock Paper Scissors game just so you could see the same project from another fellow programmer's perspective.



                                import java.util.Arrays;
                                import java.util.Collections;
                                import java.util.List;
                                import java.util.Scanner;

                                /**
                                * Created on 7/4/2016.
                                *
                                */

                                public class RockPaperScissors {
                                public static void main(String args){
                                List<Integer> computerMoves = Arrays.asList(1, 2, 3); // Used to generate random number.
                                Scanner input = new Scanner(System.in);

                                // Infinite loop until broken by exit case.
                                loop : while(true){
                                System.out.print("Rock, Paper, Scissors, Exit >>> ");
                                String move = input.nextLine().toLowerCase();
                                Collections.shuffle(computerMoves); // Shuffles list to achieve random numbers.

                                switch(move){
                                case "rock" : determineWinner(computerMoves.get(0), "Tie", "You Lose", "You Win"); break; // if move is rock
                                case "paper" : determineWinner(computerMoves.get(0), "You Win", "Tie", "You Lose"); break; // if move is paper
                                case "scissors" : determineWinner(computerMoves.get(0), "You Lose", "You Win", "Tie"); break; // if move is scissors
                                case "exit" : break loop; // if move is exit
                                case "" : continue loop; // if move is blank
                                default : System.out.println("Invalid Input"); // if move is none of the above
                                }
                                }
                                }

                                // Used in the interest of code re-usability
                                private static void determineWinner(int computerMove, String m1, String m2, String m3){
                                if(computerMove == 1){ // computer move is rock
                                System.out.printf("Computer Chose Rock, %s%n", m1);
                                } else if(computerMove == 2){ // computer move is paper
                                System.out.printf("Computer Chose Paper, %s%n", m2);
                                } else { // computer move is scissors
                                System.out.printf("Computer Chose Scissors, %s%n", m3);
                                }
                                }
                                }


                                Now, whether my coding decisions were the absolute best for this situation or not, there are still a lot of things you can take from my code. I tried to reuse some of the same tools that you used so you wouldn't be too lost but I also tried to implement some new things you might have never used or seen before.



                                Formatting:



                                Take note of the curly braces in my code, they emphasize the actual "blocks" of code. Your code is very hard to read because of poorly placed braces. Here is a link to another review, the top answer goes a lot more in depth on curly braces placement and has some decent visuals as well. Also while on the topic of indentation you have a few silly indentations through out.



                                    Random rnd = new Random();
                                System.out.println("Rock Paper Scissors, by Sage!");
                                System.out.println("Select 1, 2, 3, for Rock, Paper, Scissors");
                                //Menu Present, pretty bad still


                                And



                                input = sage.nextInt();

                                if(input == yes){
                                System.out.println("Rock,Paper,Scissors!");

                                }else{
                                System.out.println("Thanks for playing!");
                                }


                                Indentations are generally only followed by open curly braces. Not sure if these were just copy paste issues but worth mentioning.



                                Functionality:



                                This line stands out the most I believe,



                                input = sage.nextInt();


                                It accepts the next Integer, if given anything other than an integer your application will crash. Most of the time its a good idea to have some sort of check in there and correct a user or at least stop them from crashing the program. I deal with this by accepting a string and checking all expected input with a switch case statement. If a user gives this application something other than the 5 expected inputs, it will correct them by saying "Invalid Input".



                                Another notable problem with your game is that it will never stop. int b in your rps class never changes. When asking the player if they want to continue, you will have to change int b so that it equals 0 for the game to stop. Because int b is in another class from rps2, you would reference it like so:



                                rps.b = 0;


                                Conclusion



                                These are just a few important things to keep in mind. I'm sure someone will touch on some of the others.






                                share|improve this answer














                                I decided to write my own little Rock Paper Scissors game just so you could see the same project from another fellow programmer's perspective.



                                import java.util.Arrays;
                                import java.util.Collections;
                                import java.util.List;
                                import java.util.Scanner;

                                /**
                                * Created on 7/4/2016.
                                *
                                */

                                public class RockPaperScissors {
                                public static void main(String args){
                                List<Integer> computerMoves = Arrays.asList(1, 2, 3); // Used to generate random number.
                                Scanner input = new Scanner(System.in);

                                // Infinite loop until broken by exit case.
                                loop : while(true){
                                System.out.print("Rock, Paper, Scissors, Exit >>> ");
                                String move = input.nextLine().toLowerCase();
                                Collections.shuffle(computerMoves); // Shuffles list to achieve random numbers.

                                switch(move){
                                case "rock" : determineWinner(computerMoves.get(0), "Tie", "You Lose", "You Win"); break; // if move is rock
                                case "paper" : determineWinner(computerMoves.get(0), "You Win", "Tie", "You Lose"); break; // if move is paper
                                case "scissors" : determineWinner(computerMoves.get(0), "You Lose", "You Win", "Tie"); break; // if move is scissors
                                case "exit" : break loop; // if move is exit
                                case "" : continue loop; // if move is blank
                                default : System.out.println("Invalid Input"); // if move is none of the above
                                }
                                }
                                }

                                // Used in the interest of code re-usability
                                private static void determineWinner(int computerMove, String m1, String m2, String m3){
                                if(computerMove == 1){ // computer move is rock
                                System.out.printf("Computer Chose Rock, %s%n", m1);
                                } else if(computerMove == 2){ // computer move is paper
                                System.out.printf("Computer Chose Paper, %s%n", m2);
                                } else { // computer move is scissors
                                System.out.printf("Computer Chose Scissors, %s%n", m3);
                                }
                                }
                                }


                                Now, whether my coding decisions were the absolute best for this situation or not, there are still a lot of things you can take from my code. I tried to reuse some of the same tools that you used so you wouldn't be too lost but I also tried to implement some new things you might have never used or seen before.



                                Formatting:



                                Take note of the curly braces in my code, they emphasize the actual "blocks" of code. Your code is very hard to read because of poorly placed braces. Here is a link to another review, the top answer goes a lot more in depth on curly braces placement and has some decent visuals as well. Also while on the topic of indentation you have a few silly indentations through out.



                                    Random rnd = new Random();
                                System.out.println("Rock Paper Scissors, by Sage!");
                                System.out.println("Select 1, 2, 3, for Rock, Paper, Scissors");
                                //Menu Present, pretty bad still


                                And



                                input = sage.nextInt();

                                if(input == yes){
                                System.out.println("Rock,Paper,Scissors!");

                                }else{
                                System.out.println("Thanks for playing!");
                                }


                                Indentations are generally only followed by open curly braces. Not sure if these were just copy paste issues but worth mentioning.



                                Functionality:



                                This line stands out the most I believe,



                                input = sage.nextInt();


                                It accepts the next Integer, if given anything other than an integer your application will crash. Most of the time its a good idea to have some sort of check in there and correct a user or at least stop them from crashing the program. I deal with this by accepting a string and checking all expected input with a switch case statement. If a user gives this application something other than the 5 expected inputs, it will correct them by saying "Invalid Input".



                                Another notable problem with your game is that it will never stop. int b in your rps class never changes. When asking the player if they want to continue, you will have to change int b so that it equals 0 for the game to stop. Because int b is in another class from rps2, you would reference it like so:



                                rps.b = 0;


                                Conclusion



                                These are just a few important things to keep in mind. I'm sure someone will touch on some of the others.







                                share|improve this answer














                                share|improve this answer



                                share|improve this answer








                                edited Apr 13 '17 at 12:40









                                Community

                                1




                                1










                                answered Jul 5 '16 at 4:26









                                JSextonn

                                406211




                                406211












                                • Appreciate it a ton! Format is definitely something I need to work on as it is horribly messy but the indentation rule of open curly braces will help more than you could imagine for me. I haven't even began to touch on strings yet. A lot of stuff in your program has been stuff I have not touched on but do understand to some degree, I love seeing it from another perspective. Once again, thank you for the input =).
                                  – PtSage
                                  Jul 5 '16 at 4:34










                                • I edited a few more comments in there to try and clear some things up. Happy coding !
                                  – JSextonn
                                  Jul 5 '16 at 4:41


















                                • Appreciate it a ton! Format is definitely something I need to work on as it is horribly messy but the indentation rule of open curly braces will help more than you could imagine for me. I haven't even began to touch on strings yet. A lot of stuff in your program has been stuff I have not touched on but do understand to some degree, I love seeing it from another perspective. Once again, thank you for the input =).
                                  – PtSage
                                  Jul 5 '16 at 4:34










                                • I edited a few more comments in there to try and clear some things up. Happy coding !
                                  – JSextonn
                                  Jul 5 '16 at 4:41
















                                Appreciate it a ton! Format is definitely something I need to work on as it is horribly messy but the indentation rule of open curly braces will help more than you could imagine for me. I haven't even began to touch on strings yet. A lot of stuff in your program has been stuff I have not touched on but do understand to some degree, I love seeing it from another perspective. Once again, thank you for the input =).
                                – PtSage
                                Jul 5 '16 at 4:34




                                Appreciate it a ton! Format is definitely something I need to work on as it is horribly messy but the indentation rule of open curly braces will help more than you could imagine for me. I haven't even began to touch on strings yet. A lot of stuff in your program has been stuff I have not touched on but do understand to some degree, I love seeing it from another perspective. Once again, thank you for the input =).
                                – PtSage
                                Jul 5 '16 at 4:34












                                I edited a few more comments in there to try and clear some things up. Happy coding !
                                – JSextonn
                                Jul 5 '16 at 4:41




                                I edited a few more comments in there to try and clear some things up. Happy coding !
                                – JSextonn
                                Jul 5 '16 at 4:41










                                up vote
                                1
                                down vote













                                General




                                1. Represent expressive game elements in extra classes (Players, Rock/Scissor/Paper, Result, Situation, ...) for OOP

                                2. Try to reduce case handling, use proper let structures work for you

                                3. Follow JAVA conventions in Naming


                                Code



                                I looked after a solution with less if statements and more expressive artifacts. I came up with this:



                                Introduce a class "Situation"



                                This represents a game situation with the choice of each player:



                                public class Situation {


                                private final String choice1;
                                private final String choice2;


                                public Situation(String choice1, String choice2) {
                                this.choice1 = choice1;
                                this.choice2 = choice2;
                                }


                                @Override
                                public int hashCode() {
                                return choice1.hashCode() * choice2.hashCode();
                                }


                                @Override
                                public boolean equals(Object obj) {

                                boolean equals = false;

                                if (obj instanceof Situation) {

                                Situation that = (Situation) obj;

                                equals = this.choice1.equals(that.choice1)
                                && this.choice2.equals(that.choice2);

                                }

                                return equals;
                                }


                                public Situation invert() {
                                return new Situation(choice2, choice1);
                                }


                                }


                                It's a value object so recognize the immutability and hashcode/equals overriden.



                                One other thing is the invert method which is a convenience method to express the other side around.



                                You may consider to use "real" objects to represent "Rock", Scissor" and "Paper" instead of String-Objects.



                                Introduce class "Result"



                                It represents the outcome of one round.



                                public enum Result {

                                TIE, PLAYER1_WINS, PLAYER2_WINS

                                }


                                Evaluate all possible situations



                                You can now easily provide a Set of possible winning situations to evaluate the winner.



                                public class RockScissorPaper {


                                private Set<Situation> winSituations;


                                public RockScissorPaper() {

                                this.winSituations = new HashSet<>();

                                this.winSituations.add(new Situation("Rock", "Scissor"));
                                this.winSituations.add(new Situation("Scissor", "Paper"));
                                this.winSituations.add(new Situation("Paper", "Rock"));

                                }


                                public Result evaluateWinner(Situation situation) {

                                if (this.winSituations.contains(situation)) {
                                return Result.PLAYER1_WINS;
                                } else if (this.winSituations.contains(situation.invert())) {
                                return Result.PLAYER2_WINS;
                                } else {
                                return Result.TIE;
                                }

                                }


                                public static void main(String args) {

                                RockScissorPaper rockScissorPaper = new RockScissorPaper();

                                System.out.println(rockScissorPaper.evaluateWinner(new Situation("Rock", "Rock"))); // --> TIE
                                System.out.println(rockScissorPaper.evaluateWinner(new Situation("Scissor", "Paper"))); // --> PLAYER1_WINS
                                System.out.println(rockScissorPaper.evaluateWinner(new Situation("Rock", "Paper"))); // --> PLAYER2_WINS

                                }


                                }





                                share|improve this answer

























                                  up vote
                                  1
                                  down vote













                                  General




                                  1. Represent expressive game elements in extra classes (Players, Rock/Scissor/Paper, Result, Situation, ...) for OOP

                                  2. Try to reduce case handling, use proper let structures work for you

                                  3. Follow JAVA conventions in Naming


                                  Code



                                  I looked after a solution with less if statements and more expressive artifacts. I came up with this:



                                  Introduce a class "Situation"



                                  This represents a game situation with the choice of each player:



                                  public class Situation {


                                  private final String choice1;
                                  private final String choice2;


                                  public Situation(String choice1, String choice2) {
                                  this.choice1 = choice1;
                                  this.choice2 = choice2;
                                  }


                                  @Override
                                  public int hashCode() {
                                  return choice1.hashCode() * choice2.hashCode();
                                  }


                                  @Override
                                  public boolean equals(Object obj) {

                                  boolean equals = false;

                                  if (obj instanceof Situation) {

                                  Situation that = (Situation) obj;

                                  equals = this.choice1.equals(that.choice1)
                                  && this.choice2.equals(that.choice2);

                                  }

                                  return equals;
                                  }


                                  public Situation invert() {
                                  return new Situation(choice2, choice1);
                                  }


                                  }


                                  It's a value object so recognize the immutability and hashcode/equals overriden.



                                  One other thing is the invert method which is a convenience method to express the other side around.



                                  You may consider to use "real" objects to represent "Rock", Scissor" and "Paper" instead of String-Objects.



                                  Introduce class "Result"



                                  It represents the outcome of one round.



                                  public enum Result {

                                  TIE, PLAYER1_WINS, PLAYER2_WINS

                                  }


                                  Evaluate all possible situations



                                  You can now easily provide a Set of possible winning situations to evaluate the winner.



                                  public class RockScissorPaper {


                                  private Set<Situation> winSituations;


                                  public RockScissorPaper() {

                                  this.winSituations = new HashSet<>();

                                  this.winSituations.add(new Situation("Rock", "Scissor"));
                                  this.winSituations.add(new Situation("Scissor", "Paper"));
                                  this.winSituations.add(new Situation("Paper", "Rock"));

                                  }


                                  public Result evaluateWinner(Situation situation) {

                                  if (this.winSituations.contains(situation)) {
                                  return Result.PLAYER1_WINS;
                                  } else if (this.winSituations.contains(situation.invert())) {
                                  return Result.PLAYER2_WINS;
                                  } else {
                                  return Result.TIE;
                                  }

                                  }


                                  public static void main(String args) {

                                  RockScissorPaper rockScissorPaper = new RockScissorPaper();

                                  System.out.println(rockScissorPaper.evaluateWinner(new Situation("Rock", "Rock"))); // --> TIE
                                  System.out.println(rockScissorPaper.evaluateWinner(new Situation("Scissor", "Paper"))); // --> PLAYER1_WINS
                                  System.out.println(rockScissorPaper.evaluateWinner(new Situation("Rock", "Paper"))); // --> PLAYER2_WINS

                                  }


                                  }





                                  share|improve this answer























                                    up vote
                                    1
                                    down vote










                                    up vote
                                    1
                                    down vote









                                    General




                                    1. Represent expressive game elements in extra classes (Players, Rock/Scissor/Paper, Result, Situation, ...) for OOP

                                    2. Try to reduce case handling, use proper let structures work for you

                                    3. Follow JAVA conventions in Naming


                                    Code



                                    I looked after a solution with less if statements and more expressive artifacts. I came up with this:



                                    Introduce a class "Situation"



                                    This represents a game situation with the choice of each player:



                                    public class Situation {


                                    private final String choice1;
                                    private final String choice2;


                                    public Situation(String choice1, String choice2) {
                                    this.choice1 = choice1;
                                    this.choice2 = choice2;
                                    }


                                    @Override
                                    public int hashCode() {
                                    return choice1.hashCode() * choice2.hashCode();
                                    }


                                    @Override
                                    public boolean equals(Object obj) {

                                    boolean equals = false;

                                    if (obj instanceof Situation) {

                                    Situation that = (Situation) obj;

                                    equals = this.choice1.equals(that.choice1)
                                    && this.choice2.equals(that.choice2);

                                    }

                                    return equals;
                                    }


                                    public Situation invert() {
                                    return new Situation(choice2, choice1);
                                    }


                                    }


                                    It's a value object so recognize the immutability and hashcode/equals overriden.



                                    One other thing is the invert method which is a convenience method to express the other side around.



                                    You may consider to use "real" objects to represent "Rock", Scissor" and "Paper" instead of String-Objects.



                                    Introduce class "Result"



                                    It represents the outcome of one round.



                                    public enum Result {

                                    TIE, PLAYER1_WINS, PLAYER2_WINS

                                    }


                                    Evaluate all possible situations



                                    You can now easily provide a Set of possible winning situations to evaluate the winner.



                                    public class RockScissorPaper {


                                    private Set<Situation> winSituations;


                                    public RockScissorPaper() {

                                    this.winSituations = new HashSet<>();

                                    this.winSituations.add(new Situation("Rock", "Scissor"));
                                    this.winSituations.add(new Situation("Scissor", "Paper"));
                                    this.winSituations.add(new Situation("Paper", "Rock"));

                                    }


                                    public Result evaluateWinner(Situation situation) {

                                    if (this.winSituations.contains(situation)) {
                                    return Result.PLAYER1_WINS;
                                    } else if (this.winSituations.contains(situation.invert())) {
                                    return Result.PLAYER2_WINS;
                                    } else {
                                    return Result.TIE;
                                    }

                                    }


                                    public static void main(String args) {

                                    RockScissorPaper rockScissorPaper = new RockScissorPaper();

                                    System.out.println(rockScissorPaper.evaluateWinner(new Situation("Rock", "Rock"))); // --> TIE
                                    System.out.println(rockScissorPaper.evaluateWinner(new Situation("Scissor", "Paper"))); // --> PLAYER1_WINS
                                    System.out.println(rockScissorPaper.evaluateWinner(new Situation("Rock", "Paper"))); // --> PLAYER2_WINS

                                    }


                                    }





                                    share|improve this answer












                                    General




                                    1. Represent expressive game elements in extra classes (Players, Rock/Scissor/Paper, Result, Situation, ...) for OOP

                                    2. Try to reduce case handling, use proper let structures work for you

                                    3. Follow JAVA conventions in Naming


                                    Code



                                    I looked after a solution with less if statements and more expressive artifacts. I came up with this:



                                    Introduce a class "Situation"



                                    This represents a game situation with the choice of each player:



                                    public class Situation {


                                    private final String choice1;
                                    private final String choice2;


                                    public Situation(String choice1, String choice2) {
                                    this.choice1 = choice1;
                                    this.choice2 = choice2;
                                    }


                                    @Override
                                    public int hashCode() {
                                    return choice1.hashCode() * choice2.hashCode();
                                    }


                                    @Override
                                    public boolean equals(Object obj) {

                                    boolean equals = false;

                                    if (obj instanceof Situation) {

                                    Situation that = (Situation) obj;

                                    equals = this.choice1.equals(that.choice1)
                                    && this.choice2.equals(that.choice2);

                                    }

                                    return equals;
                                    }


                                    public Situation invert() {
                                    return new Situation(choice2, choice1);
                                    }


                                    }


                                    It's a value object so recognize the immutability and hashcode/equals overriden.



                                    One other thing is the invert method which is a convenience method to express the other side around.



                                    You may consider to use "real" objects to represent "Rock", Scissor" and "Paper" instead of String-Objects.



                                    Introduce class "Result"



                                    It represents the outcome of one round.



                                    public enum Result {

                                    TIE, PLAYER1_WINS, PLAYER2_WINS

                                    }


                                    Evaluate all possible situations



                                    You can now easily provide a Set of possible winning situations to evaluate the winner.



                                    public class RockScissorPaper {


                                    private Set<Situation> winSituations;


                                    public RockScissorPaper() {

                                    this.winSituations = new HashSet<>();

                                    this.winSituations.add(new Situation("Rock", "Scissor"));
                                    this.winSituations.add(new Situation("Scissor", "Paper"));
                                    this.winSituations.add(new Situation("Paper", "Rock"));

                                    }


                                    public Result evaluateWinner(Situation situation) {

                                    if (this.winSituations.contains(situation)) {
                                    return Result.PLAYER1_WINS;
                                    } else if (this.winSituations.contains(situation.invert())) {
                                    return Result.PLAYER2_WINS;
                                    } else {
                                    return Result.TIE;
                                    }

                                    }


                                    public static void main(String args) {

                                    RockScissorPaper rockScissorPaper = new RockScissorPaper();

                                    System.out.println(rockScissorPaper.evaluateWinner(new Situation("Rock", "Rock"))); // --> TIE
                                    System.out.println(rockScissorPaper.evaluateWinner(new Situation("Scissor", "Paper"))); // --> PLAYER1_WINS
                                    System.out.println(rockScissorPaper.evaluateWinner(new Situation("Rock", "Paper"))); // --> PLAYER2_WINS

                                    }


                                    }






                                    share|improve this answer












                                    share|improve this answer



                                    share|improve this answer










                                    answered Jul 5 '16 at 8:01









                                    oopexpert

                                    3,029416




                                    3,029416

















                                        protected by Jamal 33 mins ago



                                        Thank you for your interest in this question.
                                        Because it has attracted low-quality or spam answers that had to be removed, posting an answer now requires 10 reputation on this site (the association bonus does not count).



                                        Would you like to answer one of these unanswered questions instead?



                                        Popular posts from this blog

                                        Ellipse (mathématiques)

                                        Quarter-circle Tiles

                                        Mont Emei