1
\$\begingroup\$

For my computer science class I am tasked with writing a rock/paper/scissors game. I understand how to write the game, but I was wondering if someone more experienced than I could give me some pointers on how to become a better programmer.

Is there anything I could do to reduce the size of the code, but without diminishing functionality?

public class RockPaperScissors 
{
    static double wins = 0;
    static double losses = 0;
    static double draws = 0;
    static double games = 0;

    private static void lose() {
        System.out.println("YOU LOSE! :D");
        losses++;
        games++;
    }

    private static void win() {
        System.out.println("you win :D");
        wins++;
        games++;
    }

    // main method
    public static void main(String[] args) 
    {
        Scanner scan = new Scanner(System.in);

        // variables
        Integer answer1 = 0;
        boolean playing = true;

        // start of game
        do 
        {
            // main menu
            System.out.println("Input R, P, S, or Q(to quit).");

            // player's move
            String answer = scan.next();
            answer = answer.toLowerCase();

            // converts player's move from str to int
            switch (answer.charAt(0))
            {
            case 'r':
                answer1 = 0;
                break;
            case 'p':
                answer1 = 1;
                break;
            case 's':
                answer1 = 2;
                break;
            case 'q':
                System.out.println("User exit.");
                double winP = wins/games*100;
                double loseP = losses/games*100;
                double drawP = draws/games*100;
                System.out.println("Wins: " + wins);
                System.out.printf("%.2f", winP);
                System.out.println(" Percent");
                System.out.println("Losses: " + losses);
                System.out.printf("%.2f", loseP);
                System.out.println(" Percent");
                System.out.println("Draws: " + draws);
                System.out.printf("%.2f", drawP);
                System.out.println(" Percent");
                playing = false;
                break;
            default: 
                System.out.println("ERROR!!");
                playing = false;
                break;
            }

            // checks for conditions
            if (playing) 
            {
                // computer's move
                Random random = new Random();
                Integer choice = random.nextInt(3); // (n-1) 0 = rock, 1 = paper, 2 = scissors
                String comp = choice.toString();
                switch (comp)
                {
                case "0":
                    System.out.println("computer chose rock");
                    break;
                case "1":
                    System.out.println("computer chose paper");
                    break;
                case "2":
                    System.out.println("computer chose scissors");
                    break;
                default:
                    System.out.println("ERROR!");
                    break;
                }
                // checks results
                if (answer1 == choice)
                {
                    System.out.println("Draw");
                    draws++;
                    games++;
                }
                else if ((answer1 + 1) % 3 == choice)
                    lose();
                else if ((choice + 1) % 3 == answer1)
                    win();
                else
                    System.out.println("ERROR! ERROR!");
            }           
        } while (playing);
    }
}
\$\endgroup\$
2
\$\begingroup\$

What do you mean by "reduce the size"? Code or byte-code? If it's code you can easily remove a few lines by merging your System.out.print*():

System.out.printf("Wins: %d %.2f%%\nLosses: %d %.2f%%\nDraws %d %.2f%%",
    wins, winP, losses, loseP, draws, drawP);

Also, what is the use of transforming Integer into a String for the switch()? switch() on String was introduced in Java 7. A better-optimized version would be to use int directly:

int choice = random.nextInt(3); // (n-1) 0 = rock, 1 = paper, 2 = scissors
switch (choice) {
    case 0:
        ...
}

You can also have a String[] to store what you want to display:

String[] rps = { "rock", "paper", "scissors" };
if (choice >= 0 && choice < rps.length)
    System.out.println("Computer chose "+rps[choice]);
else
    System.out.println("Wrong choice!");
// Or use ternary operator:
//System.out.println(choice >= 0 && choice < rps.length ? "Computer chose "+rps[choice] : "Wrong choice!");

For byte-code size you can use primitive int instead of class Integer. And also int for your wins, losses, draws and games variables: ++ incrementation is a lot faster on int than on double. You can convert them to double only when computing the percentage (and correct a potential percentage calculation bug when using integer arithmetics):

loseP = 100.0 * losses / games; // 100.0 is a double, so 100.0 * losses is a double and (100.0 * losses) / games is also a double
// loseP = losses/games*100 will always be an int: losses and games are int, losses/games is an int, 100 is an int and losses/games*100 is an int

I'll let you figure out the rest :)

\$\endgroup\$
  • \$\begingroup\$ This was very insightful! I never even considered using an array. I'm going to spend some time going back and looking over these suggestions \$\endgroup\$ – cookiez Oct 25 '15 at 2:06
  • \$\begingroup\$ @cookiez enjoy! :) \$\endgroup\$ – Matthieu Oct 25 '15 at 6:40
2
\$\begingroup\$

Also extract new methods - userQuit(), checkResult(), etc. Then your main game loop will fit the screen, will be easier to maintain, and prettier.

Avoid useless comments like //main method when you extract self described methods you will see that you don't need comments.

\$\endgroup\$
1
\$\begingroup\$

You can code cleaner, small and reliable with some minimal optimizations, but as long as it remains in main mode will be an imperative program, not an Object Oriented. Object orientation make the aplication scalable with ease.

In the following code you can see that it implements the classic infinite main loop, it is not an error since System.exit calls can break all loops, switch or functions. Also added some code to handle the case when user want to quit without playing.

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

/**
 * @author cookiez
 */
public class RockPaperScissors {

    static int wins = 0;
    static int losses = 0;
    static int draws = 0;
    static int games = 0;

    private static void lose() {
        System.out.println("YOU LOSE! :D");
        losses++;
    }

    private static void win() {
        System.out.println("you win :D");
        wins++;
    }

    private static void draw() {
        System.out.println("Draw");
        draws++;
    }

    // main method
    public static void main(String[] args) {
        Scanner scan = new Scanner(System.in);

        // variables
        int user_choice = 0;
        Random random = new Random();
        String answer;
        int computer_choice;

        // start of game
        do {
            // main menu
            System.out.println("Input R, P, S, or Q(to quit).");

            // player's move
            answer = scan.next().toLowerCase();

            // converts player's move from str to int
            switch (answer.charAt(0)) {
                case 'r':
                    user_choice = 0;
                    break;
                case 'p':
                    user_choice = 1;
                    break;
                case 's':
                    user_choice = 2;
                    break;
                case 'q':
                    System.out.println("User exit.");
                    if (games > 0) {
                        double winP = (double) wins / games * 100;
                        double loseP = (double) losses / games * 100;
                        double drawP = (double) draws / games * 100;
                        System.out.printf("Wins: %d \n%.2f  Percent\nLosses: %d\n%.2f Percent\nDraws: %d\n%.2f Percent", wins, winP, losses, loseP, draws, drawP);
                    }
                    System.exit(0);
                default:
                    System.err.println("ERROR!!");
                    System.exit(1);
            }

            // computer's move
            computer_choice = random.nextInt(3); // (n-1) 0 = rock, 1 = paper, 2 = scissors
            switch (computer_choice) {
                case 0:
                    System.out.println("computer chose rock");
                    break;
                case 1:
                    System.out.println("computer chose paper");
                    break;
                case 2:
                    System.out.println("computer chose scissors");
                    break;
                default:
                    System.out.println("ERROR!");
                    break;
            }
            // checks results
            if (user_choice == computer_choice) {
                draw();
            } else if ((user_choice + 1) % 3 == computer_choice) {
                lose();
            } else {
                win();
            }
            games++;
        } while (true);
    }
}
\$\endgroup\$
  • \$\begingroup\$ Welcome to Code Review! Good job on your first answer. \$\endgroup\$ – SirPython Oct 22 '15 at 21:57
  • \$\begingroup\$ Tnx... sorry for my bad english. \$\endgroup\$ – Ruslan López Oct 22 '15 at 22:31
1
\$\begingroup\$

This answer focuses on the negative code paths.

You use "ERROR!!", "ERROR!", and "ERROR! ERROR!" to indicate something has gone wrong but don't provide any information as to what went wrong or how to fix it. It's trivial is this example but a good habit to adopt.

The first ERROR!! is in the user's control. Reiterate the valid input. Maybe even echo the invalid input that caused the error.

The second error is outside of the user's control, and shouldn't happen, but if it does then you can help yourself debug it by stating the expected random value was outside the range 0-2 and print the unexpected value.

The last error is another case that shouldn't happen; you can only win, lose, or draw a game. But if that path occurs I would print the choice and answer1 in the error message.

\$\endgroup\$
  • \$\begingroup\$ Welcome to Code Review! Good job on your first answer. \$\endgroup\$ – SirPython Oct 22 '15 at 23:37

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.