8
\$\begingroup\$

I wrote a Rock Paper Scissors game and was hoping to get some feedback on issues like:

  • Am I utilizing object oriented techniques correctly? What can I do better?
  • Am I using main() correctly? My understanding is that it should be a main controller for the rest of the program. Should I be utilizing a counter in a different method, instead of in main?
  • Am I using my instance variables correctly?
  • I realize that the computer choice is skewed. How should I do it differently?
  • The way I check for wins seems a little convoluted. Should I be using a different system to check?
  • What did I do well, so that I can remember to use those techniques? What did I do poorly?

Main

import java.util.*;
public class driver {



public static void main(String[] args) {

    int userWins = 0, computerWins = 0;

    System.out.println("Welcome to Rocks, Paper, Scissors. How many games would you like to play?");
    Scanner in = new Scanner(System.in);
    int numberOfGames = in.nextInt();
    int userCounter = 0;
    int computerCounter = 0;
    for (int i = 0; i < numberOfGames; i++) {
        RPS Game = new RPS();
        String user = Game.startGame();
        String computer = Game.computerChoice();
        boolean wins = Game.evaluateGame(user, computer);
        if (wins == true){
            userWins++;
        }
        else {
            computerWins++;
        }
        System.out.println("Number of wins:");
        System.out.println("Computer: " + computerWins);
        System.out.println("User: " + userWins);
        System.out.println();
    }
  }
}

RPS

import java.util.Scanner;

public class RPS {
    String comparableChoice;
    String userComparableChoice;


        public String startGame() {
        Scanner in = new Scanner(System.in);
        System.out.println("Choose, (1)Rock, (2)Paper or (3)Scissors");
        System.out.println("enter number: 1,2 or 3");
        int userChoice = in.nextInt();

        if (userChoice == 1){
            System.out.println("Your choice: Rock");
            userComparableChoice = "Rock";
        }
        else if (userChoice == 2){
            System.out.println("Your choice: Paper");
            userComparableChoice = "Paper";
        }
        else{
            System.out.println("Your choice: Scissors");
            userComparableChoice = "Scissors";
        }
        return userComparableChoice;
    }

    public String computerChoice(){
        int computerChoiceNumber = (int)(Math.random() * 10);
        if (computerChoiceNumber <= 3){
            System.out.println("Computer choice: Rock");
            comparableChoice = "Rock";
        }
        else if (computerChoiceNumber <= 6){
            System.out.println("Computer choice: Paper");
            comparableChoice = "Paper";
        }
        else{
            System.out.println("Computer choice: Scissors");
            comparableChoice = "Scissors";
        }
        return comparableChoice;
    }

    public boolean evaluateGame(String userFormal, String computerFormal){

        if (userFormal.equals(computerFormal)){
            System.out.println("It's a tie");

        }
        if (userFormal.equalsIgnoreCase("Rock") && computerFormal.equalsIgnoreCase("Scissors")){
            System.out.println("You won!");
            return true;
        }
        if (userFormal.equalsIgnoreCase("Paper") && computerFormal.equalsIgnoreCase("Rock")){
            System.out.println("You won");
            return true;
        }
        if (userFormal.equalsIgnoreCase("Scissors") && computerFormal.equalsIgnoreCase("Paper")){
            System.out.println("You won");
            return true;
        }
        else {
            System.out.println("Computer won");
            return false;
        }

    }

}
\$\endgroup\$
7
\$\begingroup\$

I would only call the method startGame() inside the main function and handle from there the entire logic of the game. The main method shouldn't be aware of the exact procedure of a game. It just wants to start a game and wants to know who won. As an output from that method you could return true if the user wins and false if the computer wins. If you want to handle a tie separately you can return an int value instead for indicating the outcome (-1 computer wins, 0 draw, 1 user wins).

Inside the startGame() method you can call the game logic functions/your algorithm. In pseudo code it would look sth like this:

public int startGame()
{
    userChoice();
    computerChoice();
    return evaluateWinner(); // Evaluate winner and returns -1, 0, 1 depending on the user and computer choice (i.e. who the winner is or if its a tie)
}

You can make the other methods of the RPS class private to have a clean interface for this class, allowing other classes only to start a game and prevent them interfering the game algorithm itself.

\$\endgroup\$
6
\$\begingroup\$
  • if (wins == true) is a tautology. Preferred way is if (wins). You may also consider renaming wins to more descriptive userWon.

  • It seems you do not account for draws.

  • Determining a winner is convoluted indeed. Using numbers (0,1,2) instead of strings Rock,Scissor,Paper you may notice that the difference modulo 3 being 0 means a draw, 1 means a win for second, and -1 means a win for the first.

\$\endgroup\$
  • \$\begingroup\$ Ah ok wins == true is redundant. Using a modulo seems very succinct and exactly the kind of solution I was seeking. Thank you very much! \$\endgroup\$ – Michael James Feb 4 '15 at 1:23
5
\$\begingroup\$

I'll just answer two of your questions so far:

Am I utilizing object oriented techniques correctly? What can I do better?

You can utilize more classes, such as a Move class. This would make it easier to add additional moves if desired. You could also have actual Player and Computer classes (not just Strings) to allow easier modifications for their functionalities.

Essentially, if someone has to comb through your code to figure out where to start making modifications, then something should likely be improved. This is especially important if you'd like others to play and/or maintain your game.

Am I using main() correctly?

That's a good start, at least. Perhaps the loop code can be moved elsewhere, but it still doesn't seem to be doing much more than is needed. You could primarily have it create instances and retrieving the outcome. The most important thing is that the core functionality is not crammed into main(), which you have indeed started doing.

One note about driver: it should also be capitalized since it is a class.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.