Rock, Paper, Scissors, Lizard and Spock

Question

I am currently done with this little Rock, Paper, Scissors, Lizard, Spock game in JavaScript. The game is working fine, but I am not pleased with the code. Is there a way to refactor these if statements? I was thinking about ternary operators and I tried doing so but failed. This is basically my function based on which I get the results.

function result(userChoice, cpuChoice) {
    var result = '';

     if(userChoice == 'rock' ) {
        if (cpuChoice == 'rock') {
            result = 'Tie';
            ties++;
        } else if ( cpuChoice == 'spock') {
            result = 'Spock vaporizes rock';
            loses++;
        } else if ( cpuChoice == 'lizard' ) {
            result = 'Rock crushes lizard';
            wins++;
        }  else if ( cpuChoice == 'paper' ) {
            result = 'Paper covers rock';
            loses++;
        }  else if ( cpuChoice == 'scissors' ) {
            result = 'Rock crushes scissors';
            wins++;
        };
    } else if(userChoice == 'paper') {
        if (cpuChoice == 'paper') {
            result = 'Tie';
            ties++;
        } else if ( cpuChoice == 'spock') {
            result = 'Paper disproves Spock';
            wins++;
        } else if ( cpuChoice == 'lizard' ) {
            result = 'Lizard eats paper';
            loses++;
        }  else if ( cpuChoice == 'rock' ) {
            result = 'Paper covers rock';
            wins++;
        }  else if ( cpuChoice == 'scissors' ) {
            result = 'Scissors cuts paper';
            loses++;
        };
    } else if(userChoice == 'scissors') {
        if (cpuChoice == 'scissors') {
            result = 'Tie';
            ties++;
        } else if ( cpuChoice == 'spock') {
            result = 'Spock distroys scissors';
            loses++;
        } else if ( cpuChoice == 'lizard' ) {
            result = 'Scissors beheads lizard';
            wins++;
        }  else if ( cpuChoice == 'rock' ) {
            result = 'Rock crushes scissors';
            loses++;
        }  else if ( cpuChoice == 'paper' ) {
            result = 'Scissors cuts paper';
            wins++;
        };  
    } else if(userChoice == 'lizard') {
        if (cpuChoice == 'lizard') {
            result = 'Tie';
            ties++;
        } else if ( cpuChoice == 'spock') {
            result = 'Lizard poisons Spock';
            wins++;
        } else if ( cpuChoice == 'scissors' ) {
            result = 'Scissors beheads lizard' ;
            loses++;
        }  else if ( cpuChoice == 'rock' ) {
            result = 'Rock crushes lizard';
            loses++;
        }  else if ( cpuChoice == 'paper' ) {
            result = 'Lizard eats paper';
            wins++;
        };  
    } else if(userChoice == 'spock') {
        if (cpuChoice == 'spock') {
            result = 'Tie';
            ties++;
        } else if ( cpuChoice == 'lizard') {
            result = 'Lizard poisons Spock';
            loses++;
        } else if ( cpuChoice == 'scissors' ) {
            result = 'Spock distroys scissors';
            wins++;
        }  else if ( cpuChoice == 'rock' ) {
            result = 'Spock vaporizes rock';
            wins++;
        }  else if ( cpuChoice == 'paper' ) {
            result = 'Paper disproves Spock';
            loses++;
        };  
    } else {
        return false;
    };

    return result;
};

You may not be aware of it, but Rock Paper Scissors Lizard Spock was an old "community challenge" around here, which is why I added the relevant tag to your post. You may be interested in reading other RPSLS submissions to the challenge. This one in particular is a nice approach: RPSLS by Mat's Mug. — Simon Forsberg, Aug 27, 2015 at 10:56
Additionally, there was a PCG challenge for it as well with some pretty good answers. — XNargaHuntress, Aug 28, 2015 at 17:59

Quill · Accepted Answer · 2015-08-27 23:48:10Z

To review the code as it is:

It would be better not to even have result as a variable, and swap the order of operations inside the else-ifs that assign result from (result = X, gameResult++) to (gameResult++, return X)
You don't need semi-colons on the end of if statements:

} else {
    return false;
};

Rather than individually testing (userInput == X, then X == cpuInput), remove them and add the following if statement to the top.

if (userChoice === cpuChoice){
    ties++;
    return "Tie";
}
//The rest follows

Onto fixing the `if-else`!

So, instead of a long if-else statement, I'd use an object, which lets you specify key and value pairs, which really cleans the process up, because you don't have to iterate over all the options before finding the one you need.

Here's what I came up with:

var results = {
    wins: 0,
    loses: 0,
    ties: 0
};

function RPSLZ(userChoice, cpuChoice) {
    var RULES = {
        rock: {
            lizard: 'Rock crushes lizard',
            scissors: 'Rock crushes scissors'
        },
        paper: {
            spock: 'Paper disproves Spock',
            rock: 'Paper covers rock'
        },
        scissors: {
            lizard: 'Scissors beheads lizard',
            paper: 'Scissors cuts paper'
        },
        lizard: {
            spock: 'Lizard poisons Spock',
            paper: 'Lizard eats paper'
        },
        spock: {
            scissors: 'Spock distroys scissors',
            rock: 'Spock vaporizes rock'
        }
    };
    if (userChoice == cpuChoice) {
        results.ties++;
        return 'Tie';
    } else if (!userChoice in RULES){
        return 'Invalid Input';
    } else {
        return (cpuChoice in RULES[userChoice]
                    ? (results.wins++, RULES[userChoice][cpuChoice])
                    : (results.loses++, RULES[cpuChoice][userChoice])
                );
    }
}

How does it work?

That's a good question. It follows the following logic flow:

Declare the results outside the function, so it can continue recording results with more function calls (There is a way to do this inside the function, but I couldn't figure out how.)
Declaring the game RULES (Notice the all-caps, meaning it's a constant)
There's two special rules: tie and Invalid Input. They can be tested like described above, and like testing whether the input exists inside the RULES variable, because if it didn't, it would be undefined.
By using the same undefined testing rule, the cpuChoice can be tested to see whether it's inside the object of the userChoice, depending on the result, the respective counter is incremented, and the result returned.

You shouldn't use typeof RULES[userChoice] === 'undefined'. You should use the in operator. — Ismael Miguel, Aug 27, 2015 at 14:25

Community · Accepted Answer · 2017-04-13 12:40:52Z

I've posted this answer for review on: Rock, Paper, Scissors, Lizard and Spock with OOP

One thing that I quickly noticed is the repetitiveness. One that highly ticked me off is the amount of tries. You could use an early return, like this:

if(userChoice == cpuChoice) {
    tries++;
    return 'Tie';
}

By doing that, you can clean up the ties!

You have misspelled destroys twice.

For your code to work, you have to know what's what.

You can try to create a fully Object-Oriented thing:

function RockPaperScissors(name, hard_action, soft_action) {
    this.name = (name + '').toLowerCase();
    this.hard_action = hard_action + '';
    this.soft_action = (soft_action || hard_action) + '';
    this.strengths = {};
    this.weaknesses = {};
}

RockPaperScissors.prototype = {
    getName: function(){
        return this.name;
    },
    addStrength: function(strength) {
        if( !(strength instanceof RockPaperScissors) ) {
            throw new TypeError('A strength must be an instance of RockPaperScissors');
        }

        this.strengths[strength.getName()] = strength;
    },
    getStrengths: function() {
        var strengths = {};
        for(var k in this.strengths)
        {
            strengths[k] = this.strengths[k];
        }
        return strengths;
    },
    addWeakness: function(weakness, soft) {
        if( !(weakness instanceof RockPaperScissors) ) {
            throw new TypeError('A weakness must be an instance of RockPaperScissors');
        }

        this.weaknesses[weakness.getName()] = {
            enemy: weakness,
            soft: !!soft
        };
    },
    getWeaknesses: function() {
        var weaknesses = {};
        for(var k in this.weaknesses)
        {
            weaknesses[k] = this.weaknesses[k];
        }
        return weaknesses;
    },
    getAction(soft){
        return soft? this.soft_action : this.hard_action;
    },
    winsTo: function(enemy) {
        if( !(enemy instanceof RockPaperScissors) ) {
            throw new TypeError('An enemy must be an instance of RockPaperScissors');
        }

        return (enemy.getName() in this.strengths);
    }
};

function RockPaperScissorsGame(){
    this.score = {
        won: 0,
        lost: 0,
        tied: 0
    };
    this.games = 0;
}

RockPaperScissorsGame.prototype = {
    match: function(user, enemy){
        if( !(user instanceof RockPaperScissors) ) {
            throw new TypeError('The user must be an instance of RockPaperScissors');
        }
        if( !(enemy instanceof RockPaperScissors) ) {
            throw new TypeError('The enemy must be an instance of RockPaperScissors');
        }

        this.games++;

        if( user == enemy ) {
            this.score.tied++;
            return 'Tie';
        }

        var userName = user.getName();
        var enemyName = enemy.getName();

        if( user.winsTo(enemy) ) {
            this.score.won++;
            var enemyWeakness = enemy.getWeaknesses()[userName] || {enemy: enemy, soft: false};

            return userName + ' ' + user.getAction(enemyWeakness.soft) + ' ' + enemyName;

        } else {
            this.score.lost++;
            var userWeakness = user.getWeaknesses()[enemyName] || {enemy: enemy, soft: false};

            return enemyName + ' ' + enemy.getAction(userWeakness.soft) + ' ' + userName;
        }
    },
    getMatches: function(){
        return this.games;
    },
    getWins: function(){
        return this.won;
    },
    getLosses: function(){
        return this.lost;
    },
    getTies: function(){
        return this.tied;
    }
};

This allows for more abstraction, since you don't have to know that, in a specific case, this wins to that and all the conditions.

You run a method, on a new game, and it does everything for you. You just have to worry to capitalizing it properly. You simply have to create the elements, add strengths and weaknesses and you're done!

To run it, you simply can do like this:

function RockPaperScissors(name, hard_action, soft_action) {
	this.name = (name + '').toLowerCase();
	this.hard_action = hard_action + '';
	this.soft_action = (soft_action || hard_action) + '';
	this.strengths = {};
	this.weaknesses = {};
}

RockPaperScissors.prototype = {
	getName: function(){
		return this.name;
	},
	addStrength: function(strength) {
		if( !(strength instanceof RockPaperScissors) ) {
			throw new TypeError('A strength must be an instance of RockPaperScissors');
		}
		
		this.strengths[strength.getName()] = strength;
	},
	getStrengths: function() {
		var strengths = {};
		for(var k in this.strengths)
		{
			strengths[k] = this.strengths[k];
		}
		return strengths;
	},
	addWeakness: function(weakness, soft) {
		if( !(weakness instanceof RockPaperScissors) ) {
			throw new TypeError('A weakness must be an instance of RockPaperScissors');
		}
		
		this.weaknesses[weakness.getName()] = {
			enemy: weakness,
			soft: !!soft
		};
	},
	getWeaknesses: function() {
		var weaknesses = {};
		for(var k in this.weaknesses)
		{
			weaknesses[k] = this.weaknesses[k];
		}
		return weaknesses;
	},
	getAction(soft){
		return soft? this.soft_action : this.hard_action;
	},
	winsTo: function(enemy) {
		if( !(enemy instanceof RockPaperScissors) ) {
			throw new TypeError('An enemy must be an instance of RockPaperScissors');
		}
		
		return (enemy.getName() in this.strengths);
	}
};

function RockPaperScissorsGame(){
	this.score = {
		won: 0,
		lost: 0,
		tied: 0
	};
	this.games = 0;
}

RockPaperScissorsGame.prototype = {
	match: function(user, enemy){
		if( !(user instanceof RockPaperScissors) ) {
			throw new TypeError('The user must be an instance of RockPaperScissors');
		}
		if( !(enemy instanceof RockPaperScissors) ) {
			throw new TypeError('The enemy must be an instance of RockPaperScissors');
		}
		
		this.games++;
		
		if( user == enemy ) {
			this.score.tied++;
			return 'Tie';
		}
		
		var userName = user.getName();
		var enemyName = enemy.getName();
		
		if( user.winsTo(enemy) ) {
			this.score.won++;
			var enemyWeakness = enemy.getWeaknesses()[userName] || {enemy: enemy, soft: false};
			
			return userName + ' ' + user.getAction(enemyWeakness.soft) + ' ' + enemyName;
			
		} else {
			this.score.lost++;
			var userWeakness = user.getWeaknesses()[enemyName] || {enemy: enemy, soft: false};
			
			return enemyName + ' ' + enemy.getAction(userWeakness.soft) + ' ' + userName;
		}
	},
	getMatches: function(){
		return this.games;
	},
	getWins: function(){
		return this.score.won;
	},
	getLosses: function(){
		return this.score.lost;
	},
	getTies: function(){
		return this.score.tied;
	}
};


var rock = new RockPaperScissors('rock', 'crushes');
var paper = new RockPaperScissors('paper', 'wraps', 'disproves');
var scissor = new RockPaperScissors('scissor', 'cuts', 'beheads');

scissor.addStrength(paper);
rock.addStrength(scissor);
paper.addStrength(rock);

var game = new RockPaperScissorsGame();

alert([
	game.match(rock,paper),
	game.match(rock,rock),
	game.match(scissor,paper),
	game.getWins(),
	game.getLosses(),
	game.getTies(),
	game.getMatches()
].join('\r\n'));

By doing this, you abstract the winning conditions and the gameplay, easing the programming, readability, mantainability and flexibility. You can add as many enemies as you like, with all sorts of strengths and weaknesses.

SuperBiasedMan · Accepted Answer · 2015-08-27 10:52:07Z

Use Javascript's switch statement instead of long if else chains. It basically allows you to pass in one parameter ie. userChoice or cpuChoice and then run different code based on what value it is.

    switch (cpuChoice) {
        case 'rock':
            result = 'Tie';
            ties++;
            break;
        case 'spock':
            result = 'Spock vaporizes rock';
            loses++;
            break;
        case 'lizard':
            result = 'Rock crushes lizard';
            wins++;
            break;
        case 'paper':
            result = 'Paper covers rock';
            loses++;
            break;
        case 'scissors':
            result = 'Rock crushes scissors';
            wins++;
            break;
    }

You can also nest switch statements to put switches inside a switch's case block. You can also use default: as your last case, if none of the other cases evaluated as true.

I will try and see if I can do it your way as well, I find it´s a good practice to try several ways to get to the same result. Cheers! Will post the code here after I am pleased with it :D. — Daria M, Aug 27, 2015 at 11:48

Chimera · Accepted Answer · 2015-08-27 16:40:45Z

You can get rid of the if-else entirely if you imagine the objects being on a wheel in a particular order. You then figure out who wins by comparing the distance each symbol is away from the other (negative, equal or positive).

Here is some Python Code:

I know this is tagged JavaScript, but it's easy enough to translate to JavaScript.

# The key idea of this program is to equate the strings
# "rock", "paper", "scissors", "lizard", "Spock" to numbers
# as follows:
#
# 0 - rock
# 1 - Spock
# 2 - paper
# 3 - lizard
# 4 - scissors

import random

# helper functions

def number_to_name(number):
    """
    Converts a number to its name equivalent
    and returns said name
    """

    if number == 0:
        return "rock"
    elif number == 1:
        return "Spock"
    elif number == 2:
        return "paper"
    elif number == 3:
        return "lizard"
    elif number == 4:
        return "scissors"
    else:
        return "That is not a valid number!"


def name_to_number(name):
    """
    Converts a name to its number equivalent
    and returns said number
    """

    if name == "rock":
        return 0
    elif name == "Spock":
        return 1
    elif name == "paper":
        return 2
    elif name == "lizard":
        return 3
    elif name == "scissors":
        return 4
    else:
        return "That is not a valid name!"


def rpsls(name): 

    # convert name to player_number
    player_number = name_to_number(name)

    # compute random guess for comp_number
    comp_number = random.randrange(0, 4)

    # compute difference of player_number and comp_number modulo five
    difference = (player_number - comp_number) % 5

    # determine winner
    if difference == 0:
        winner = "It's a tie!"
    elif difference <= 2:
        winner = "Player wins!"
    elif difference >= 3:
        winner = "Computer wins!"
    else:
        return "Something went horribly wrong"

    # convert comp_number to name using number_to_name
    comp_name = number_to_name(comp_number)

    # print results
    print ""
    print "Player chooses", name
    print "Computer chooses", comp_name
    print winner


# test the code
rpsls("rock")
rpsls("Spock")
rpsls("paper")
rpsls("lizard")
rpsls("scissors")

please include code that you wish to be part of your answer, a link can be added as well, but keep in mind that links sometimes rot. — Malachi, Aug 27, 2015 at 15:56
Python and Javascript are different languages. One of the differences is the print statement. There's no such thing in Javascript. There is, however, the document.writeln() but that is a bad practice. — Ismael Miguel, Aug 27, 2015 at 16:18
The key point is the method to determine who wins... Not the stupid way to print the winner. — Chimera, Aug 27, 2015 at 16:21
You can easily remove those number_to_name() and name_to_number() functions with arrays (using array.indexOf() for the reverse lookup) — IQAndreas, Aug 28, 2015 at 7:03
@IQAndreas Thanks. You are correct. I wasn't focusing on other optimizations other than showing a way to avoid if-else or lookup methods to determine the winner of a game. — Chimera, Aug 28, 2015 at 19:51

James · Accepted Answer · 2015-08-28 07:37:33Z

To "flatten" the rules, and avoid nested switch statements, try something like:

switch (userChoice + cpuChoice) {
  case "rockscissors":
    msg = "Rock crushes scissors";
    wins++;
    break;
  case "scissorsrock":
    msg = "Rock crushes scissors";
    losses++;
    break;    
  case "spockrock":
    msg = "Spock vaporizes rock";
    wins++;
    break;
// etc
}

Why? It's a lot easier to have one rule entry per atomic rule. You could put all the possible responses into an object, and have a much more generic function to calculate the outcome:

var rules = {
  rockscissors: {
    msg: "Rock crushes scissors", score: 1
  },
  scissorsrock: {
    msg: "Rock crushes scissors", score: -1
  },
  spockspock: {
    msg: "Tie", score: 0
  }
};

var r = rules[userChoice + cpuChoice];

if (r) {
  msg = r.msg;
  wins += r.score == 1 ? 1 : 0;
  losses += r.score == -1 ? 1 : 0;
  ties += r.score == 0 ? 1 : 0;
}

Mawg says reinstate Monica · Accepted Answer · 2016-12-22 09:09:43Z

1

Despite there being an already accepted answer with 14 upvotes, I feel obliged to point out this this program is screaming out for a lookup table.

This code should be data driven and could possibly be reduced to a single line (certainly to just a few).

Why not ...

if (superior[spock][lizard])

to show what I mean (it is always false).

In your case

if (superior[userChoice][cpuChoice])

I leave it as an exercise for the reader to hard code the array superior[][], and to build an apporpriate text string.

edited Dec 22, 2016 at 9:09

answered Aug 28, 2015 at 8:10

Mawg says reinstate Monica

3571 silver badge12 bronze badges

Add a comment |

Stack Exchange Network

Rock, Paper, Scissors, Lizard and Spock

6 Answers 6

Onto fixing the `if-else`!

Your Answer

Not the answer you're looking for? Browse other questions tagged
javascript
game
community-challenge
rock-paper-scissors
or ask your own question.

Linked

Hot Network Questions

Rock, Paper, Scissors, Lizard and Spock

6 Answers 6

Onto fixing the if-else!

Your Answer

Sign up or log in

Post as a guest

Not the answer you're looking for? Browse other questions tagged javascriptgamecommunity-challengerock-paper-scissors or ask your own question.

Linked

Related

Hot Network Questions

Onto fixing the `if-else`!

Not the answer you're looking for? Browse other questions tagged
javascript
game
community-challenge
rock-paper-scissors
or ask your own question.