Rock-Paper-Scissors (etc) with encapsulated business logic

Question

Overview

The following is an attempt at a Rock-Paper-Scissors (etc) game that has its business logic encapsulated in a rules engine. For the purposes of this exercise, I kept all of the business logic (eg what-beats-what and the "verbs") in a JBoss Drools engine.

This application doesn't have an "interface" per-se. It's merely the business logic and some utility classes.

Principles

The following were the principles I tried to follow:

1. Extensibility. A business-y person should be able to extend the game without having to modify the code. (In this case, I considered the "rules" to be non-code.)
2. Simple. If a business-y person needed to extend the code (see previous), they should only have to make minimal changes.

Implementation

As proof of concept, I implemented the rules of Rock-Paper-Scissors-Lizard-Spock, using the verbs supplied by the Sheldon Cooper quote:

"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."

-- Dr. Sheldon Cooper

Intended use

As can be seen by the code I've included below, there is no actual interface to the business logic. The eventual goal is to stick an HTTP interface on top of this logic and to allow POST requests to play a game of Rock-Paper-Scissors-Etc.

Technology

As mentioned, I used JBoss Drools to implement the business logic, using the mvel syntax. The supporting code, which consists of simple POJOs and a utility class for invoking the rules, is in Java. To validate existing behavior, I used the BDD testing library Cucumber and invoked it using JUnit. The project itself is a Maven project for ease of building.

The full code, as it stands, can be found on GitHub here: https://github.com/roddy/rock-paper-scissors-etc

Code

There are four Java classes: Player, ValidationOutcome, GameOutcome, and DrlUtilities. The first three are POJOs, the last is a utility class for invoking the rules.

Player.java

package net.roddy.rps.assets;

public class Player {
    private String myMove;
    public void setMove(String myMove) {
        this.myMove = myMove;
    }
    public String getMove() {
        return myMove;
    }
}

ValidationOutcome.java

package net.roddy.rps.assets;

import java.util.ArrayList;
import java.util.List;

public class ValidationOutcome {

  private boolean isValid;
  private List<String> errors;

  public ValidationOutcome() {
    this.isValid = true;
    this.errors = new ArrayList<>();
  }
  public void setValid(boolean isValid) {
    this.isValid = isValid;
  }
  public boolean isValid() {
    return this.isValid;
  }
  public void addValidationError(String error) {
    this.errors.add(error);
  }
  public List<String> getValidationErrors() {
    return new ArrayList<>(errors);
  }
}

GameOutcome.java

package net.roddy.rps.assets;

public class GameOutcome {

  private String winningThrow;
  private String verb;
  private String losingThrow;

  public void setWinningThrow(String winningThrow) {
    this.winningThrow = winningThrow;
  }
  public String getWinningThrow() {
    return winningThrow;
  }
  public void setLosingThrow(String losingThrow) {
    this.losingThrow = losingThrow;
  }
  public String getLosingThrow() {
    return losingThrow;
  }
  public void setVerb(String verb) {
    this.verb = verb;
  }
  public String getVerb() {
    return verb;
  }
}

DrlUtilities.java

package net.roddy.rps.utilities;

import org.kie.api.KieBase;
import org.kie.api.KieServices;
import org.kie.api.runtime.KieContainer;
import org.kie.api.runtime.KieSession;

import java.util.List;

/**
 * Utilty class for deploying DRL files and invoking rules.
 */
public class DrlUtilities {

    /**
     * Fires rules for the specified scope. Passes into the session the specified input objects, and adds a global with
     * the specified name.
     * @param scope the scope
     * @param inputs the input objects
     * @param globalName the global's name
     * @param global the global
     */
    public static void fireRulesForScope(String scope, List<Object> inputs, String globalName, Object global) {
        KieServices kieServices = KieServices.Factory.get();
        KieContainer kContainer = kieServices.getKieClasspathContainer();

        KieBase kBase = kContainer.getKieBase(scope);
        KieSession session = kContainer.newKieSession(scope+"Session");
        try {
            for(Object input : inputs) {
                session.insert(input);
            }
            session.setGlobal(globalName, global);
            session.fireAllRules();
        } finally {
            session.dispose();
        }
    }
}

In addition to the Java code, there are two DRL files (Drools) and one XML file that defines the KIE Modules.

validation.drl

package rules.validation;

import net.roddy.rps.assets.Player;
import net.roddy.rps.assets.ValidationOutcome;

dialect "mvel"

global ValidationOutcome outcome;

rule "A player's move must be all lower case"
when
  not(Player( move matches "[a-z]+"))
then
  outcome.setValid(false);
  outcome.addValidationError("Player move is not all lowercase.");
end

rule "A player's move must be one of the expected values"
when
  Player( $move : move not in ( "rock", "paper", "scissors", "lizard", "spock" ))
then
  outcome.setValid(false);
  outcome.addValidationError("Player move is invalid: " + $move);
end

competition.drl

package rules.game;

import net.roddy.rps.assets.GameOutcome;
import net.roddy.rps.assets.Player;

dialect "mvel"

global GameOutcome outcome;

rule "Duplicate throws are a tie"
when
  Player( $throw : move != null )
  Player( move == $throw )
then
  outcome.setWinningThrow($throw);
  outcome.setLosingThrow($throw);
  outcome.setVerb("ties");
end

rule "Scissors cut paper"
when
  Player( move == "scissors" )
  Player( move == "paper" )
then
  outcome.setWinningThrow("scissors");
  outcome.setVerb("cut");
  outcome.setLosingThrow("paper");
end

rule "Paper covers rock"
when
  Player( move == "paper" )
  Player( move == "rock" )
then
  outcome.setWinningThrow("paper");
  outcome.setVerb("covers");
  outcome.setLosingThrow("rock");
end

rule "Rock crushes lizard"
when
  Player( move == "rock" )
  Player( move == "lizard" ) 
then
  outcome.setWinningThrow("rock");
  outcome.setVerb("crushes");
  outcome.setLosingThrow("lizard");
end

rule "Lizard poisons Spock"
when
  Player( move == "lizard" )
  Player( move == "spock" ) 
then
  outcome.setWinningThrow("lizard");
  outcome.setVerb("poisons");
  outcome.setLosingThrow("spock");
end

rule "Spock smashes scissors"
when
  Player( move == "spock" )
  Player( move == "scissors" ) 
then
  outcome.setWinningThrow("spock");
  outcome.setVerb("smashes");
  outcome.setLosingThrow("scissors");
end

rule "Scissors decapitate lizard"
when
  Player( move == "scissors" )
  Player( move == "lizard" ) 
then
  outcome.setWinningThrow("scissors");
  outcome.setVerb("decapitate");
  outcome.setLosingThrow("lizard");
end

rule "Lizard eats paper" 
when
  Player( move == "lizard" )
  Player( move == "paper" )
then
  outcome.setWinningThrow("lizard");
  outcome.setVerb("eats");
  outcome.setLosingThrow("paper");
end

rule "Paper disproves Spock"
when
  Player( move == "paper" )
  Player( move == "spock" )
then
  outcome.setWinningThrow("paper");
  outcome.setVerb("disproves");
  outcome.setLosingThrow("spock");
end

rule "Spock vaporizes rock"
when
  Player( move == "spock" )
  Player( move == "rock" )
then
  outcome.setWinningThrow("spock");
  outcome.setVerb("vaporizes");
  outcome.setLosingThrow("rock");
end

rule "Rock crushes scissors"
when
  Player( move == "rock" )
  Player( move == "scissors" )
then
  outcome.setWinningThrow("rock");
  outcome.setVerb("crushes");
  outcome.setLosingThrow("scissors");
end

kmodule.xml

<kmodule xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns="http://jboss.org/kie/6.0.0/kmodule">
  <kbase name="validation" packages="rules.validation">
    <ksession name="validationSession" type="stateful" />
  </kbase>
  <kbase name="game" packages="rules.game">
    <ksession name="gameSession" type="stateful" />
  </kbase>
</kmodule>

Concerns

For the most part, the business logic is entirely encapsulated by the competition.drl rules. The only exception is the logic that validates that the move is acceptable. Since the validation logic is independent of the actual game logic, I had to include the list of actions in both places. So if we decide to add a new action -- "jedi" -- we not only have to update the competition.drl to include the appropriate interactions, but also the validation.drl to update the list of allowed actions.

Otherwise, please feel free to comment on anything else. The project was built on Java 8, but I haven't made use of any Java-8-specific features.

Answer 1

Conceptual Framework for Domain Model

In your application, there's a rule imposed (via your Drools XML) that there is a fixed set of values to a Player's move. My opinion here is that your domain model should reflect that requirement, and seeing as you have generics in your code, your application should also be able to support Java enum. I think it's better to capture this restriction on valid moves by creating an enum called Move like the following:

public enum Move {

    ROCK("rock"), PAPER("paper"), SCISSORS("scissors"), LIZARD("lizard"), SPOCK("spock");

    private String name;

    private Move(String name) {
        this.name = name;
    }

}

This way, the code structure itself imposes the rule on what values are valid. In addition, by doing this, you won't have to specify the validation rules elsewhere.

As for your Player class, I think that it should be something that captures some form of playing strategy. It can be as simple as one move player like what you have here essentially, or something more complex. The simplest modification I have for this would be to restructure it the following way:

public class Player {

    private Move move;

    public Player(Move move) {
        this.move = move;
    }

    public Move play() {
        return this.move;
    }

}

This Player has a single move strategy, which is what you set to it upon construction. Structuring the Player the way you did is, to me, a little weak because the class doesn't seem to be in charge of its own state; it seems to rely on some external component in deciding what to do next. In some way, it can be considered an anemic data model whose only purpose is to carry data instead of representing an abstraction or a concept.

Going back to my point about the Player class representing some playing strategy, we know that, conceptually, there many ways a player can play. What I suggest here is that instead of making Player a Java class, declare it to be an interface instead, like the following:

public interface Player {

    Move play();

}

To me, this is a very simple construct that captures the essence of what a Player is: an object that returns a Move when it plays. With interface like this, you can define many different types of Players, like, for instance, one that plays randomly. For example:

public class RandomPlayer implements Player {

    //corrected as per the comment below
    private final Random random = new Random();

    public Move play() {
        Move[] moves = Move.values();
        int index = random.nextInt(moves.length + 1);
        return moves[index];
    }

}

Hopefully the RandomPlayer class illustrates my point about classes being in charge of their own state, actions, etc.

Lastly, about GameOutcome, I will also say that this class is weak, and overly transparent. My idea is that once a GameOutcome object is created, it will never change. Because of this, I think that GameOutcome should be defined this way:

public class GameOutcome {

    private Move winningMove;
    private Move losingMove;

    private String verb;

    public GameOutcome(Move winningMove, Move losingMove, String verb) {
        this.winningMove = winningMove;
        this.losingMove = losingMove;
        this.verb = verb;
    }

    public Move getWinningMove() {
        return winningMove;
    }

    public Move getLosingMove() {
        return losingMove;
    }

    public String getVerb() {
        return verb;
    }

}

I don't know what verb field represents, but I kept it. However, due to the fact that it's not readily understandable, you should reconsider why it's there in the first place. In any case, notice that there are only getters for this class, and no setters. This is because, like I said, GameOutcome objects most likely won't need to change, there's no point enabling the state transitions in them.

Rules Integration

I missed a critical aspect of the problem you're trying to solve with your code, which is the rules engine primarily geared towards not-so-technical people. However, I still think that having those rules in the system does not mean we should completely forget about good OO design. I believe that rules should augment your design, not hijack it. Since having a Move enum is quite restrictive, my idea is to define it instead as a value object:

public class Move {

    private final String name;

    public Move(String name) {
        this.name = name;
    }

    public name getName() {
        return name;
    }

}

Each move has some seemingly intrinsic characteristics. In particular, each of their name should be lower case Strings, and their values are limited to some given set of words. My impression is that these rules aren't bound to change that much anyway, so why rely on the rules engine to impose them? For one, instead of checking if their names are lower case, why don't we just ensure that they always are? Instead of making it a rule, why not turn it into a basic assumption?

public Move(String name) {
    this.name = name.toLowerCase();
}

As for the restriction about what their names can be, since it's not very convenient to use enum, then I'd suggest using a plain text file of comma separated values, added in the classpath. For example a move.dat file in the src/main/resources directory could look like:

rock, paper, scissors, lizard, spock

Then from this file, create a MoveSet class that loads all the possible Moves specified:

public class MoveSet {

    private static final Map<String, Move> MOVE_MAP;

    static {
        InputStream stream = StripeCustomerTest.class.getClassLoader().getResourceAsStream("moves.dat");
        try {
            //Apache IOUtils
            String input = IOUtils.toString(stream);
            List<String> moveNames = asList(input.split(",\\p{Blank}?"));
            Map<String, Move> moveMap = new HashMap<>();
            Move move = null;
            for (String moveName : moveNames) {
                move = new Move(moveName);
                moveMap.put(move.getName(), move);
            }
            MOVE_MAP = Collections.unmodifiableMap(moveMap);
        } catch (IOException e) {
            throw new RuntimeException(e);
        }
    }

    private MoveSet() {}

    public Move getMove(String name) {
        return MOVE_MAP.get(name.toLowerCase());
    }

}

Ideally, Move class should be itself the MoveSet, but since Drools seem to rely on POJOs and, the constructor for Move can't be made private, I think this is an acceptable compromise. With this, there probably won't be a need to have separate rule for validating Moves unless there are expected, more complex checking. Also, since the application is checking an external file for values, there isn't a need to recompile or rebuild the code when a new Move like Jedi is introduced into the system.

As for the rules on game proper, I'd say they should have been applied to a Game and not to separate players. The system doesn't even seem to be interested enough in Players to give them names like "Player 1", or "Player 2"; they merely carry a Move in them. A Game contains 2 Move objects, each from 2 opponents. The fact that there is a GameOutcome class seems to suggest there should have been a Game class to begin with.

public class Game {

    private final Move player1Move;
    private final Move player2Move;

    public Game(Move player1Move, Move player2Move) {
        this.player1Move = player1Move;
        this.player2Move = player2Move;
    }

    //getters here

}

Admittedly, I don't know much about Drools, but guessing from how the rules are defined here, using this Game class, rules can be defined like (this is not Drools syntax):

rule "paper beats rock"
    player1Move.name == 'rock'
    player2Move.name == 'paper'

Hopefully there's a way to make this work both ways.

Anyway, I don't expect to get everything correctly here, but I hope somehow I raised interesting points regarding your code structure, and domain design