Rock Paper Scissors
Hi guys! One of the things that I am beginning to see is that there is always a more efficient way of writing code that makes use of more powerful java syntax. Of course, writing neat, efficient code takes practice and if everyone decided to post their code here asking for people to make sugestions on how to de-clutter and make it more efficient, then threads would be spawning everywhere. However, I would really apreciate any suggestions that would make the following code more efficient. It's a Rock paper sissors game where you type in the console Rock,Paper or Sissors and a random one of the three is assigned to the opponent. This code does not keep score or anything as you can see- in fact its because its so simple that I feel it must be able to be simplified.
By the way, I have a habit of using long names for some variables but please ignore them!
The code does work, I would just like suggestions on simplification- things I can learn from and carry into other projects.
import java.io.Console;
publicclass Main{
publicstaticvoid main(String[] args){
Console console;
if ((console= System.console()) !=null){
String tmp = console.readLine("Input rock, paper or scissors: ");
RockPaperScissors yourGo;
System.out.println(tmp);
if (tmp.equalsIgnoreCase("ROCK")){yourGo = RockPaperScissors.ROCK;}
elseif(tmp.equalsIgnoreCase("SCISSORS")){yourGo = RockPaperScissors.SCISSORS;}
else{yourGo = RockPaperScissors.PAPER;}
RockPaperScissors myGo = RockPaperScissors.getRandom();
System.out.format("On your turn you choose %s.%n", yourGo);
System.out.format("On my turn I chose %s.%n", myGo);
String theResult = String.format("%nResult: ");
if (myGo == RockPaperScissors.ROCK){
theResult = String.format("%s %s", theResult, yourGo.rock());
}elseif (myGo == RockPaperScissors.PAPER){
theResult = String.format("%s %s", theResult, yourGo.paper());
}else{
theResult = String.format("%s %s", theResult, yourGo.scissors());
}
System.out.println(theResult);
}else{
System.err.println("Couldn't get console!");
}
}
}
import java.util.Random;
/**
*
* @author user
*/
publicenum RockPaperScissors{
NO_VALUE (Result.NO_RESULT, Result.NO_RESULT, Result.NO_RESULT),
ROCK(Result.DRAW, Result.LOSE, Result.WIN),
PAPER(Result.WIN, Result.DRAW, Result.LOSE),
SCISSORS (Result.LOSE, Result.WIN, Result.DRAW);
/*These variables point to the results of each
*constant versus Rock,Paper and Scissors
*/
privatefinal Result vRock;
privatefinal Result vPaper;
privatefinal Result vScissors;
RockPaperScissors(Result rock, Result paper, Result scissors){
this.vRock = rock;
this.vPaper = paper;
this.vScissors = scissors;
}
public Result rock(){return vRock;}
public Result paper(){return vPaper;}
public Result scissors(){return vScissors;}
publicstatic Random myRandom =new Random();
publicstatic RockPaperScissors getRandom(){
int RPS = myRandom.nextInt(3) + 1;
return RockPaperScissors.values()[RPS];
}
privateenum Result{
NO_RESULT, WIN, LOSE, DRAW
}
}
Is using the enum varibles to hold the result of each possible outcome the best way to do things?
Really would apreciate any and all suggestions!
[6078 byte] By [
Kazeha] at [2007-11-27 11:54:41]
I'd have done it this way. But there isn't really anything technically wrong with yours.
import java.util.Random;
import java.util.Scanner;
public class RPSGame {
private enum RPS {
ROCK, PAPER, SCISSORS;
public final static RPS fromOrdinal(int ordinal) {
if (ordinal == ROCK.ordinal()) {
return ROCK;
} else if (ordinal == PAPER.ordinal()) {
return PAPER;
} else {
return SCISSORS;
}
}
public final static Result getResult(RPS player, RPS comp) {
if (player.equals(comp)) {
return Result.DRAW;
}
switch (player) {
default:
return Result.NO_RESULT;
case ROCK:
if (comp.equals(RPS.SCISSORS)) {
return Result.WIN;
}
return Result.LOSE;
case PAPER:
if (comp.equals(RPS.ROCK)) {
return Result.WIN;
}
return Result.LOSE;
case SCISSORS:
if (comp.equals(RPS.ROCK)) {
return Result.WIN;
}
return Result.LOSE;
}
}
}
private enum Result {
NO_RESULT, WIN, LOSE, DRAW
}
public void play() {
Scanner console = new Scanner(System.in);
while (true) {
System.out.println("enter R,P,S");
String userChoice = console.nextLine();
RPS yourGo;
if (userChoice.startsWith("R") || userChoice.startsWith("r")) {
yourGo = RPS.ROCK;
} else if (userChoice.startsWith("S") || userChoice.startsWith("s")) {
yourGo = RPS.SCISSORS;
} else if (userChoice.startsWith("P") || userChoice.startsWith("p")) {
yourGo = RPS.PAPER;
} else if (userChoice.equalsIgnoreCase("x")) {
System.out.println("Exiting");
break;
} else {
System.out.println("Invalid entry");
continue;
}
RPS myGo = RPSGenerator.getRandom();
System.out.format("On your turn you choose %s.%n", yourGo);
System.out.format("On my turn I chose %s.%n", myGo);
String theResult = String.format("%nResult: ");
theResult = String.format("%s %s", theResult, RPS.getResult(yourGo, myGo));
System.out.println(theResult);
}
}
final static class RPSGenerator {
private static final Random RNG = new Random();
public static RPS getRandom() {
return RPS.fromOrdinal(RNG.nextInt(3) + 1);
}
}
public static void main(String[] args) {
RPSGame game = new RPSGame();
game.play();
}
}
> Is using the enum varibles to hold the result of each
> possible outcome the best way to do things?
Using enums is definitely the way to go here. One thing I've done in this situation is to use a payoff matrix to efficiently find a solution for each game without having a bunch of if statements or a switch:
mport java.util.Random;
enum RPS
{
ROCK("rock"),
PAPER("paper"),
SCISSORS("scissors");
private String text;
private static Random rand = new Random();
// this payoff matrix helps decide who wins in all possible
// game situations
private final static int[][] PAYOFF_MATRIX =
{
{0, -1, 1}, // RR, RP, RS
{1, 0, -1}, // PR, PP, PS
{-1, 1, 0}// SR, SP, SS
};
private RPS(String text)
{
this.text = text;
}
/**
* finds winner of individual rock paper scissors game
* @param r1 RPS variable 1
* @param r2 RPS variable 2
* @return 1 if r1 beats r2; -1 if r2 beats r1; 0 if tie
*/
public static int getWinner(RPS r1, RPS r2)
{
int index1 = r1.ordinal();
int index2 = r2.ordinal();
return PAYOFF_MATRIX[index1][index2];
}
public String getText()
{
return this.text;
}
@Override
public String toString()
{
return getText();
}
public static RPS getRpsIgnoreCase(String text)
{
RPS[] rpsArray = RPS.values();
for (int i = 0; i < rpsArray.length; i++)
{
if (text.equalsIgnoreCase(rpsArray[i].getText()))
{
return rpsArray[i];
}
}
return null;
}
public static RPS getRandom()
{
int randInt = rand.nextInt(RPS.values().length);
return RPS.values()[randInt];
}
/**
* main method to test concepts of this class. Not to be used
* in the "production" version.
*/
public static void main(String[] args)
{
for (int i = 0; i < RPS.values().length; i++)
{
System.out.println(RPS.values()[i]);
}
for (int i = 0; i < RPS.values().length; i++)
{
for (int j = 0; j < RPS.values().length; j++)
{
RPS rps1 = RPS.values()[i];
RPS rps2 = RPS.values()[j];
try
{
int result = RPS.getWinner(rps1, rps2);
//rps1.myCompareTo(rps2);
System.out.println(rps1 + " compared to " + rps2 + " = " + result);
}
catch (Exception e)
{
e.printStackTrace();
}
}
}
}
}
> > Is using the enum varibles to hold the result of
> each
> > possible outcome the best way to do things?
>
> Using enums is definitely the way to go here.
> One thing I've done in this situation is to use a
> payoff matrix to efficiently find a solution for
> each game without having a bunch of if statements or
> a switch:
>
That's nice. For anything above 3 items, the switch would have been unwieldy.
I would use more classes than you and macrules. For instance, here is my RPSPlayer class:
public class RPSPlayer
{
private RPS rps;
private int score;
public static int getWinner(RPSPlayer player1, RPSPlayer player2)
{
int result = RPS.getWinner(player1.rps, player2.rps);
if (result > 0)
{
player1.incrScore();
}
else if (result < 0)
{
player2.incrScore();
}
return result;
}
public int getWinner(RPSPlayer player2)
{
return RPSPlayer.getWinner(this, player2);
}
public RPSPlayer() {
rps = null;
score = 0;
}
public RPS getRps()
{
return rps;
}
public void setRps(RPS rps)
{
this.rps = rps;
}
public int getScore()
{
return score;
}
public void setScore(int score)
{
this.score = score;
}
public void incrScore()
{
score++;
}
@Override
public String toString()
{
return rps.toString();
}
}
> I would use more classes than you and macrules. For
> instance, here is my RPSPlayer class:
That's just overkill :)
> That's just overkill :)
Yeah, but I'm trying to learn more about OOP and classes. So what the heck?
I'm lazy and would use less classes :)
public class Game {
private static final String[] VALID_VALUES = {"ROCK", "SCISSORS", "PAPER"};
private static final Map<String, String> WINNING_MAP = new HashMap<String, String>();
private static final Random RND = new Random();
static {
WINNING_MAP.put("ROCK", "SCISSORS");
WINNING_MAP.put("SCISSORS", "PAPER");
WINNING_MAP.put("PAPER", "ROCK");
}
public static void main(String[] args) {
Scanner console = new Scanner(System.in);
while (true) {
System.out.println("enter ROCK, PAPER or SCISSORS");
String userChoice = console.nextLine().toUpperCase();
String computerChoice = VALID_VALUES[RND.nextInt(VALID_VALUES.length)];
if (computerChoice.equals(userChoice)) {
System.out.println("It's a draw, you both had " + computerChoice);
continue;
}
if (WINNING_MAP.get(userChoice).equals(computerChoice)) {
System.out.println("You won, computer had " + computerChoice);
continue;
}
System.out.println("You lost, computer had " + computerChoice);
}
}
}
It's a very trivial game, and I wouldn't care about the design unless it was some kind of homework. I think that the posted code does the job.
Kaj
kajbja at 2007-7-29 18:57:27 >
I like Kaj's solution... It's simpler and IMHO easier to follow. The Map of winning results is elegant.
In fact... I remain to be convinced of the utility of 1.6's new fangled enum pseudoclass. I almost always find a simpler way of doing the same thing using traditional methods.
One thing I can't seem to get away from is writing (and collecting) a library of static utility classes. You could generalize out a method to get the user choice... something like:
/**
* Gets a string from the console, which must be one of the specified options.
* @pre console should be set to System.console();
* @param String prompt - printed each time we ask the user for input.
* @param String... options - the valid options for user input. MUST BE ALL UPPERCASE.
* @returns String - the users choice IN UPPCASE, else null
*/
private static String getUsersChoice(String prompt, String... options) {
if(console==null) return null;
String response = null;
do {
try {
System.out.print(Stringz.notNull(prompt)+" ("+Arrayz.join(", ",options)+") : ");
response = console.readLine().toUpperCase();
if("".equals(response)) return null;
if(Arrayz.in(response, options)) break;
System.out.println("Just press enter to exit. Please try again ...");
} catch (Exception e) {
System.out.println(e);
System.out.println("Now Play Nice! Please try again ...");
}
} (while true);
return(response);
}
which I'd put in an abstract krc.utilz.io.Console class
Hope that helps.
edited by: corlettk frog got code tags.
> edited by: corlettk frog got code tags.
What? Is a frog stealing the code tags? I think we need to inform Sun about that ;)
kajbja at 2007-7-29 18:57:27 >
It's a something I picked up from a wee bug in an old spell checker (might have been word perfect 4)... and it's stuck with me ever since.
> It's a something I picked up from a wee bug in an old
> spell checker (might have been word perfect 4)... and
> it's stuck with me ever since.
Ms Word used to replace Ericsson with Erection if you used auto correction :)
kajbja at 2007-7-29 18:57:27 >
Yup, and Cripples to Nipples... Hey I was 14 ;-)
