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);
 }
}
asked Oct 22, 2015 at 3:27
\$\endgroup\$

4 Answers 4

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 :)

answered Oct 22, 2015 at 5:28
\$\endgroup\$
2
  • \$\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\$ Commented Oct 25, 2015 at 2:06
  • \$\begingroup\$ @cookiez enjoy! :) \$\endgroup\$ Commented Oct 25, 2015 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.

Ethan Bierlein
15.9k4 gold badges60 silver badges146 bronze badges
answered Oct 22, 2015 at 19:41
\$\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);
 }
}
answered Oct 22, 2015 at 21:00
\$\endgroup\$
2
  • \$\begingroup\$ Welcome to Code Review! Good job on your first answer. \$\endgroup\$ Commented Oct 22, 2015 at 21:57
  • \$\begingroup\$ Tnx... sorry for my bad english. \$\endgroup\$ Commented Oct 22, 2015 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.

answered Oct 22, 2015 at 23:02
\$\endgroup\$
1
  • \$\begingroup\$ Welcome to Code Review! Good job on your first answer. \$\endgroup\$ Commented Oct 22, 2015 at 23:37

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.