HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Rock Paper Scissors Lizard Spock Revisited

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
scissorspaperspockrevisitedrocklizard

Problem

Continuing the spirit of the recent Weekend Challenge, here is a revised version of the RPSLS game.

This is a follow up to my previous submission:

Rock Paper Scissors Lizard Spock as a code-style and challenge

This version is revised to accommodate the following suggestions:

  • Model revised to abstract the display mechanism from the game engine.



  • Removed static method calls where appropriate



  • fixed bugs with binary search



  • included the 'verb' component of 'Rock crushes Scissors'



  • tracked the scoring inside each player rather than in the main method.



  • removed magic number abuse.



I have implemented both a console-based and GUI (Swing) based interface for the game. This is an image of the GUI. The fine-tuning of the layout is not complete, but that is not a priority for me in terms of a review (The icons here (CC-attribution) come from Wikipedia ):

To accomodate the suggestions above I have separated the logic in to a number of classes. The following are the core classes (does not include the GUI and Console interfaces). If there's any cool ideas in here that you may feel have been copied from other submissions, then you are probably right...

Weapon.java

```
package rpsls;

import java.util.HashMap;
import java.util.Map;

/**
* Set up the rules for the game.
* What are the moves, and what beats what (and how)!
*/
public enum Weapon {
Rock, Paper, Scissors, Lizard, Spock;

static {
/*
* Scissors cuts paper, paper covers rock, rock crushes lizard, lizard poisons Spock,
* Spock smashes scissors, scissors decapitate lizard, lizard eats paper,
* paper disproves Spock, Spock vaporizes rock. And as it always has, rock crushes scissors.
* Rd. Cooper
*/
Scissors.will("cut", Paper);
Paper.will("covers", Rock);
Rock.will("crushes", Lizard);
Lizard.will("poisons", Spock);
Spock.will("smashes", Scissors);
Scissors.will("decapitate", Lizard);

Solution

-
Love the way you're using HashMap to provide the strings for the items. But in the UI, how would it look if you pick Spock and computer picks Lizard? Does it swap the positions for you? Or should it say "Spock gets poisoned by Lizard"? (I'm not sure of what's the best way is to solve this myself, but it is something to think about).

-
It is more preferable to declare Lists by interface instead of implementation. So that, if you want to switch implementation, it's enough to swap at one place (the new ArrayList) instead of two.

private final ArrayList listeners = new ArrayList<>(); // Bad

private final List listeners = new ArrayList<>(); // Good


-
Your RPSLS class only contains one big main method. And I still think that it does too much. RPSLS could instead be the game class, and the main method splitted into a couple of methods within the class.

Overall, I'd say your code looks great. Well done. Good use of interfaces. Good use of different classes and methods. It's been a pleasure reading your code. A great improvement compared to the previous version.

Code Snippets

private final ArrayList<PlayerListener> listeners = new ArrayList<>(); // Bad

private final List<PlayerListener> listeners = new ArrayList<>(); // Good

Context

StackExchange Code Review Q#36438, answer score: 6

Revisions (0)

No revisions yet.