Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

So I decided to try to make a simple RPS implementation in Java 8, the following notes should be said first:

  • At a later point it should support RPSLS aswell, as any other Gesture-game variant.
  • At a later point it should support different views, for example a graphical one.
  • I have made a special ConsoleReader class which allows me to kill reading from System.in without closing the stream.

I would like comments on:

  • Everything
  • Concurrency issues
  • Variable, method and javadoc naming and wording

The code should be quite self-explanatory.

/**
 * Class that provides a console reader, from which other classes can read.
 *
 * @author Frank van Heeswijk
 */
public class ConsoleReader implements Runnable {
    /** Synchronized mapping from objects to a list of string consumers to consume input lines. */
    private final static Map<Object, List<Consumer<String>>> CONSUMER_MAP = Collections.synchronizedMap(new HashMap<>());

    /** Synchronized mapping from objects to a blocking queue of strings to read input lines while maintaining blocking behaviour. */
    private final static Map<Object, BlockingQueue<String>> BLOCKING_QUEUE_MAP = Collections.synchronizedMap(new HashMap<>());

    /**
     * Adds a consumer on the object you pass. When console input happens, the consumer will be triggered.
     * 
     * @param object    The object to which the consumers belong
     * @param consumer  The consumer to handle new input
     */
    public static void addConsumer(final Object object, final Consumer<String> consumer) {
        ensureKey(object, CONSUMER_MAP, ArrayList::new);
        CONSUMER_MAP.get(object).add(consumer);
    }

    /**
     * Removes all consumers related to the object you pass.
     * 
     * @param object    The object to which consumers might be attached
     */
    public static void removeConsumers(final Object object) {
        CONSUMER_MAP.remove(object);
    }

    @Override
    public void run() {
        Scanner scanner = new Scanner(System.in);
        while (scanner.hasNextLine()) {
            String nextLine = scanner.nextLine();
            try {
                for (Map.Entry<Object, BlockingQueue<String>> entry : BLOCKING_QUEUE_MAP.entrySet()) {
                    entry.getValue().put(nextLine);
                }
            } catch (InterruptedException ex) {
                Thread.currentThread().interrupt();
            }
            CONSUMER_MAP.forEach((obj, list) -> list.forEach(consumer -> consumer.accept(nextLine)));
        }
    }

    /**
     * Waits for a new line of input and then returns it.
     * 
     * Per object a new queue gets created, such that taking away the line from one queue does not mean that other objects cannot read it anymore.
     * 
     * @param object    The object that asks to read the next line
     * @return  The next line from System.in
     * @throws InterruptedException     If the thread has been interrupted
     */
    public static String nextLine(final Object object) throws InterruptedException {
        ensureKey(object, BLOCKING_QUEUE_MAP, LinkedBlockingQueue::new);
        BlockingQueue<String> queue = BLOCKING_QUEUE_MAP.get(object);
        String nextLine = queue.take();
        if (queue.isEmpty()) {
            BLOCKING_QUEUE_MAP.remove(object);
        }
        return nextLine;
    }

    /**
     * Ensures that the key has an empty structure attached to it.
     * 
     * @param <V>   The type of structure of Map<Object, V>
     * @param object    The key of the map
     * @param map   The map on which to ensure a value exists
     * @param supplier  A supplier to supply an empty structure
     */
    private static <V> void ensureKey(final Object object, final Map<Object, V> map, final Supplier<V> supplier) {
        if (!map.containsKey(object)) {
            map.put(object, supplier.get());
        }
    }
}

/**
 * Interface to extend when implementing actions.
 *
 * @author Frank van Heeswijk
 */
public interface Action { }

/**
 * Interface that all types of gestures should extend.
 *
 * @author Frank van Heeswijk
 */
public interface Gesture {
    /**
     * Lists from which gestures this gesture wins.
     * 
     * @return  A list describing from which gestures this gesture wins
     */
    public List<? extends Gesture> listWinsFrom();

    /**
     * Lists to which gestures this gesture ties.
     * 
     * @return  A list describing to which gestures this gesture ties
     */
    public List<? extends Gesture> listTiesTo();

    /**
     * Lists from which gestures this gesture loses.
     * 
     * @return  A list describing from which gestures this gesture loses
     */
    public List<? extends Gesture> listLosesFrom();

    /**
     * Returns whether this gesture wins from the other gesture
     * 
     * @param <G>   The concrete type of the gesture
     * @param gesture   The other gesture
     * @return  Whether this gesture wins from the other gesture
     */
    default public <G extends Gesture> boolean winsFrom(final G gesture) {
        return listWinsFrom().contains(gesture);
    }

    /**
     * Returns whether this gesture ties to the other gesture
     * 
     * @param <G>   The concrete type of the gesture
     * @param gesture   The other gesture
     * @return  Whether this gesture ties to the other gesture
     */
    default public <G extends Gesture> boolean tiesTo(final G gesture) {
        return listTiesTo().contains(gesture);
    }

    /**
     * Returns whether this gesture loses from the other gesture
     * 
     * @param <G>   The concrete type of the gesture
     * @param gesture   The other gesture
     * @return  Whether this gesture loses from the other gesture
     */
    default public <G extends Gesture> boolean losesFrom(final G gesture) {
        return listLosesFrom().contains(gesture);
    }
}

/**
 * An enum providing the RPS gestures.
 *
 * @author Frank van Heeswijk
 */
public enum RPSGesture implements Gesture, Action, GestureRPSRules {
    ROCK,
    PAPER,
    SCISSORS;

    @Override
    public List<RPSGesture> listWinsFrom() {
        return winMapping().get(this);
    }

    @Override
    public List<RPSGesture> listTiesTo() {
        return tieMapping().get(this);
    }

    @Override
    public List<RPSGesture> listLosesFrom() {
        return loseMapping().get(this);
    }
}

/**
 * Provides the rules of a specific gesture, meaning when one wins, ties or loses.
 *
 * @author Frank van Heeswijk
 * @param <T> The type of gesture
 */
public interface GestureRules<T extends Gesture> {
    /**
     * Returns a mapping from the gestures to which gestures they win from.
     * 
     * @return  A mapping from the gestures to which gestures they win from
     */
    public Map<T, List<T>> winMapping();

    /**
     * Returns a mapping from the gestures to which gestures they tie to.
     * 
     * @return  A mapping from the gestures to which gestures they tie to
     */
    public Map<T, List<T>> tieMapping();

    /**
     * Returns a mapping from the gestures to which gestures they lose from.
     * 
     * @return  A mapping from the gestures to which gestures they lose from
     */
    public Map<T, List<T>> loseMapping();
}

/**
 * Provides an implementation for the GestureRules when considering RPS gestures.
 *
 * @author Frank van Heeswijk
 */
public interface GestureRPSRules extends GestureRules<RPSGesture> {
    @Override
    default public Map<RPSGesture, List<RPSGesture>> winMapping() {
        Map<RPSGesture, List<RPSGesture>> mapping = new HashMap<>();
        mapping.put(ROCK, Arrays.asList(SCISSORS));
        mapping.put(PAPER, Arrays.asList(ROCK));
        mapping.put(SCISSORS, Arrays.asList(PAPER));
        return mapping;
    }

    @Override
    default public Map<RPSGesture, List<RPSGesture>> tieMapping() {
        Map<RPSGesture, List<RPSGesture>> mapping = new HashMap<>();
        mapping.put(ROCK, Arrays.asList(ROCK));
        mapping.put(PAPER, Arrays.asList(PAPER));
        mapping.put(SCISSORS, Arrays.asList(SCISSORS));
        return mapping;
    }

    @Override
    default public Map<RPSGesture, List<RPSGesture>> loseMapping() {
        Map<RPSGesture, List<RPSGesture>> mapping = new HashMap<>();
        mapping.put(ROCK, Arrays.asList(PAPER));
        mapping.put(PAPER, Arrays.asList(SCISSORS));
        mapping.put(SCISSORS, Arrays.asList(ROCK));
        return mapping;
    }
}

/**
 * Interface to extend when implementing results.
 * 
 * @author Frank van Heeswijk
 */
public interface Result { }

/**
 * Enum listing the possible results of a RPS game.
 *
 * @author Frank van Heeswijk
 */
public enum RPSResult implements Result {
    WIN,
    TIE,
    LOSS;
}

/**
 * An interface holding a player that can play any type of game.
 *
 * @author Frank van Heeswijk
 * @param <A> The type of the action
 * @param <R> The type of the result
 */
public interface Player<A extends Action, R extends Result> {
    /**
     * Processes when some player has done some action and obtained some result.
     * 
     * @param target    The player that performed the action
     * @param action    The action the player performed
     * @param result    The result of the action the player has performed
     */
    default public void onPostAction(final Player<A, R> target, final A action, final R result) { }

    /**
     * Does the action of the player.
     * 
     * @return The action the player has done
     */
    public A doAction();

    /**
     * Does the backup action of the player.
     * 
     * This will get called when the normal action has timed out.
     * 
     * @return The backup action the player has done
     */
    public A doActionBackup();

    /**
     * Performs the action of the player with a timeout on an Executor.
     * 
     * This will execute the normal action of the player waiting for a maximum specified time, if a timeout occurs then it will default to the backup action.
     * 
     * @param executor  The Executor on which the action needs to be executed
     * @param period    The time period it may take until the action gets a timeout
     * @param unit      The time unit of the period
     * @return  The action performed by the player
     */
    default public A doActionWithTimeout(final Executor executor, final int period, final TimeUnit unit) {
        FutureTask<A> futureTask = new FutureTask<>(this::doAction);
        executor.execute(futureTask);
        try {
            return futureTask.get(period, unit);
        } catch (InterruptedException ex) {
            futureTask.cancel(true);
            Thread.currentThread().interrupt();
            return doActionBackup();
        } catch (ExecutionException ex) {
            throw new IllegalStateException(ex);
        } catch (TimeoutException ex) {
            timeout();
            futureTask.cancel(true);
            return doActionBackup();
        }
    }

    /**
     * Does something when a timeout has occured.
     */
    default public void timeout() { }
}

/**
 * A player that is able to play any gesture related game.
 *
 * @author Frank van Heeswijk
 * @param <A> The type of action
 * @param <R> The type of result
 */
abstract public class GesturePlayer<A extends Action, R extends Result> implements Player<A, R> {
    /** List of actions that the player can execute. **/
    protected final List<A> actions;

    /** A Random instance. **/
    private final Random random = new Random();

    /**
     * Constructs the GesturePlayer.
     * 
     * @param actions   The list of actions that can be used
     */
    public GesturePlayer(final List<A> actions) {
        Objects.requireNonNull(actions);
        this.actions = actions;
    }

    /**
     * Returns a random action.
     * 
     * @return  A random action.
     */
    protected A getRandomAction() {
        return actions.get(random.nextInt(actions.size()));
    }
}

/**
 * A player that is able to play RPS.
 *
 * @author Frank van Heeswijk
 */
abstract public class RPSPlayer extends GesturePlayer<RPSGesture, RPSResult> {
    /** The RPSView interface. */
    protected final RPSView rpsView;

    /**
     * Constructs the RPSPlayer with the RPSView interface as argument.
     * 
     * @param rpsView   The RPS view
     */
    public RPSPlayer(final RPSView rpsView) {
        super(Arrays.asList(RPSGesture.values()));
        this.rpsView = rpsView;
    }

    @Override
    public RPSGesture doActionBackup() {
        return getRandomAction();
    }

    @Override
    public void timeout() {
        rpsView.enterGestureTimeout(this);
    }
}

/**
 * A human player that can play RPS.
 *
 * @author Frank van Heeswijk
 */
public class RPSHumanPlayer extends RPSPlayer {  
    public RPSHumanPlayer(final RPSView rpsView) {
        super(rpsView);
    }

    @Override
    public RPSGesture doAction() {
        Optional<RPSGesture> gesture;
        try {
            gesture = askInput();
        } catch (InterruptedException ex) {
            Thread.currentThread().interrupt();
            return doActionBackup();
        }
        while (!gesture.isPresent()) {
            rpsView.gestureNotRecognized(actions);
            try {
                gesture = askInput();
            } catch (InterruptedException ex) {
                Thread.currentThread().interrupt();
                return doActionBackup();
            }
        }
        return gesture.get();
    }

    /**
     * Asks for user input.
     * 
     * @return  The user input, converted to a RPS gesture.
     * @throws InterruptedException     If the method gets interrupted.
     */
    private Optional<RPSGesture> askInput() throws InterruptedException {
        rpsView.pleaseEnterGesture();
        return parseRaw(rpsView.readGestureInput());
    }

    private Optional<RPSGesture> parseRaw(final String raw) {
        List<RPSGesture> gestures = actions.stream()
                .filter(action -> action.toString().toLowerCase().startsWith(raw.toLowerCase()))
                .collect(Collectors.toList());
        return ((gestures.size() == 1) ? Optional.of(gestures.get(0)) : Optional.empty());
    }
}

/**
 * An AI player that can play RPS.
 *
 * @author Frank van Heeswijk
 */
public class RPSSimpleAIPlayer extends RPSPlayer {
    /**
     * Constructs the RPSSimpleAIPlayer.
     * 
     * @param rpsView   The RPS view
     */
    public RPSSimpleAIPlayer(final RPSView rpsView) {
        super(rpsView);
    }

    @Override
    public RPSGesture doAction() {
        return getRandomAction();
    }
}

/**
 * An interface that can be used for playing any type of game. 
 *
 * @author Frank van Heeswijk
 * @param <A> The type of action in the game
 * @param <R> The type of result in the game
 * @param <P> The type of player in the game
 */
public interface Game<A extends Action, R extends Result, P extends Player<A, R>> {
    /**
     * Adds a player to this game.
     * 
     * @param player    The new player
     */
    public void addPlayer(final P player);

    /**
     * Plays one round of the game. 
     */
    public void playRound();

    /**
     * Default implementation of playing a game.
     * 
     * This will play a game that lasts forever.
     */
    default public void playGame() {
        boolean playing = true;
        while (playing) {
            playRound();
        }
    }

    /**
     * Returns the result of a player after having played the current round.
     * 
     * @param playerActions A mapping from the players to their actions
     * @param forPlayer The player for which to determine the result
     * @param forAction The action that belongs to the player
     * @return  The result of the player for this round
     */
    public R determinePlayerResult(final Map<P, A> playerActions, final P forPlayer, final A forAction);
}

/**
 * An abstract class that provides some basic implementations of the Game.
 *
 * @author Frank van Heeswijk
 * @param <A> The type of action in the game
 * @param <R> The type of result in the game
 * @param <P> The type of player in the game
 */
abstract public class AbstractGame<A extends Action, R extends Result, P extends Player<A, R>> implements Game<A, R, P> {
    /** A list holding all players. */
    protected final List<P> players = new ArrayList<>();

    @Override
    public void addPlayer(final P player) {
        Objects.requireNonNull(player);
        players.add(player);
    }
}

/**
 * A class providing an implementation for a RPS game.
 *
 * @author Frank van Heeswijk
 */
public class RPSGame extends AbstractGame<RPSGesture, RPSResult, RPSPlayer> {
    /** An executor service to run the player calculations on, initialized to two times the available cores. */
    private final ExecutorService executorService = Executors.newFixedThreadPool(Runtime.getRuntime().availableProcessors() * 2);

    /** The RPSView interface. */
    private final RPSView rpsView;

    /**
     * Construcs the RPSGame with the RPSView interface as argument.
     * 
     * @param rpsView   The RPS view
     */
    public RPSGame(final RPSView rpsView) {
        this.rpsView = rpsView;
    }

    @Override
    public void playRound() {
        Map<RPSPlayer, RPSGesture> playerGestures = players.stream()
                .collect(Collectors.toMap(
                        player -> player, 
                        player -> player.doActionWithTimeout(executorService, 10, TimeUnit.SECONDS))
                );
        playerGestures.forEach((player, gesture) -> {
            RPSResult result = determinePlayerResult(playerGestures, player, gesture);
            players.forEach(p -> p.onPostAction(player, gesture, result));
            rpsView.processPlayerAction(player, gesture, result);
        });
    }

    @Override
    public RPSResult determinePlayerResult(final Map<RPSPlayer, RPSGesture> playerActions, final RPSPlayer forPlayer, final RPSGesture forGesture) {
        int wins = 0;
        int ties = 0;
        int losses = 0;
        for (Map.Entry<RPSPlayer, RPSGesture> entry : playerActions.entrySet()) {
            RPSPlayer player = entry.getKey();
            RPSGesture gesture = entry.getValue();
            if (player.equals(forPlayer)) {
                continue;
            }
            if (forGesture.winsFrom(gesture)) {
                wins++;
            }
            else if (forGesture.tiesTo(gesture)) {
                ties++;
            }
            else if (forGesture.losesFrom(gesture)) {
                losses++;
            }
        }
        int max = IntStream.of(wins, ties, losses).max().getAsInt();
        if (max == wins) {
            return RPSResult.WIN;
        }
        else if (max == ties) {
            return RPSResult.TIE;
        }
        else {
            return RPSResult.LOSS;
        }
    }
}

/**
 * Interface having all input/output related methods for a RPSGame.
 *
 * @author Frank van Heeswijk
 */
public interface RPSView {
    /**
     * Processes a player action.
     * 
     * @param player    The player on which the action occured
     * @param gesture   The gesture the player has used
     * @param result    The result of using that gesture in that specific round
     */
    public void processPlayerAction(final RPSPlayer player, final RPSGesture gesture, final RPSResult result);

    /**
     * Processes what happens when the gesture has not been recognized.
     * 
     * @param gestures  All available gestures
     */
    public void gestureNotRecognized(final List<RPSGesture> gestures);

    /**
     * Processes what happens when the player should provide his/her gesture.
     */
    public void pleaseEnterGesture();

    /**
     * Processes what happens when the player has a timeout.
     * 
     * @param player    The player that had received a timeout
     */
    public void enterGestureTimeout(final RPSPlayer player);

    /**
     * Processes what happens when the player needs to enter his gesture.
     * 
     * @return  The entered gesture in string format
     * @throws InterruptedException     If waiting for the gesture to be entered has been interrupted
     */
    public String readGestureInput() throws InterruptedException;
}

/**
 * Class providing the console implementation of the RPSView.
 *
 * @author Frank van Heeswijk
 */
public class ConsoleRPSView implements RPSView {
    @Override
    public void processPlayerAction(final RPSPlayer player, final RPSGesture gesture, final RPSResult result) {
        Objects.requireNonNull(player);
        Objects.requireNonNull(gesture);
        Objects.requireNonNull(result);
        if (player instanceof RPSHumanPlayer) {
            System.out.println("I played " + gesture + " and the result was a " + result);
        }
        else if (player instanceof RPSSimpleAIPlayer) {
            System.out.println("The AI played " + gesture + " and the result was a " + result);
        }
    }

    @Override
    public void gestureNotRecognized(final List<RPSGesture> gestures) {
        System.out.println("Your gesture was nog recognized or ambigious, please choose from: " + 
                String.join(", ", gestures.stream().map(RPSGesture::toString).collect(Collectors.toList())));
    }

    @Override
    public void pleaseEnterGesture() {
        System.out.println("Please enter your gesture: ");
    }

    @Override
    public String readGestureInput() throws InterruptedException {
        return ConsoleReader.nextLine(this);
    }

    @Override
    public void enterGestureTimeout(final RPSPlayer player) {
        if (player instanceof RPSHumanPlayer) {
            System.out.println("You have not entered your gesture in time.");
        } else if (player instanceof RPSSimpleAIPlayer) {
            System.out.println("The AI has not entered his gesture in time.");
        }
    }
}

/**
 * The main class
 *
 * @author Frank van Heeswijk
 */
public class RPSGameMain {
    private void init() {
        new Thread(new ConsoleReader()).start();
        RPSView consoleRPSView = new ConsoleRPSView();
        RPSGame rpsGame = new RPSGame(consoleRPSView);
        rpsGame.addPlayer(new RPSHumanPlayer(consoleRPSView));
        rpsGame.addPlayer(new RPSSimpleAIPlayer(consoleRPSView));
        rpsGame.playGame();
    }

    /**
     * @param args the command line arguments
     */
    public static void main(String[] args) {
        new RPSGameMain().init();
    }
}
share|improve this question
add comment

2 Answers 2

up vote 9 down vote accepted
  • I like your usage of Java 8. I'm learning a lot just by reading your code. I'm loving Java 8 a lot and I haven't even used it myself yet.

  • Use a GestureRule class. Take a look at The Mug's Main class and/or The Monkey's static initializer in his enum. Your current way is error prone and causes some duplicated code. Currently you can by accident create a Gesture that both wins, loses and ties another gesture. That really shouldn't be possible. I'd recommend using a method such as gesture.fight(otherGesture) that returns a RPSResult to make sure that a fight between one gesture and another cannot be both a win and a loss at the same time.

  • You're currently not using the onPostAction method, and I do wonder whether you're gonna need it at all. And if you are going to need it, it can likely be made in a different/better way. By getting rid of it / doing it differently you also get rid of the R extends Result generic part of your Player interface/class(es). An option would be to use onPostRound(Map<RPSPlayer, RPSGesture> gestures) (using only the 'A' and 'P' generics) and do a for-each on the gestures you're interested in. Because technically, players make their moves at the same time.

  • Your Action and Result interfaces only seems to be marker interfaces, you don't really need them. If you remove them it will decrease your generic dependencies and you can write:

    public interface Game<A, R, P extends Player<A, R>> {
    
  • Interface methods doesn't need to be declared public, they're public automatically.

  • In your playGame method, why do you declare a boolean variable instead of using while (true)?

  • You're using newFixedThreadPool which means that your number of threads cannot go above the specified amount. Instead consider using a cached thread pool which will create as many threads as needed while keeping a specified number of threads whether they're busy or not. By using a fixed thread pool you will likely have several threads that will never have anything to do.

  • Use a toString method in your different RPSPlayer classes or a player.isAI() method instead of using instanceof in your ConsoleRPSView. Using instanceof outside an .equals method is not good practice, it can mostly be avoided. Use polymorphism and better interfaces instead.

    System.out.println(player + " played " + gesture + " and the result was a " + result);
    ...
    if (!player.isAI())
        System.out.println("You have not entered your gesture in time.");
    
  • Typo: System.out.println("Your gesture was nog recognized - nog --> not. 'nog' means 'probably' in Swedish so if reading this in a combined English-Swedish way it'd be quite funny...

  • Your RPSGameMain.init method does more than just initializing, doesn't it? I'd recommend either splitting into two methods or rename it to start or similar.

  • Overall your code quality is good, it is easy to read and mostly easy to understand.

  • I like the way you've decoupled your "console view" from your "model" with an interface :)

share|improve this answer
add comment

As a relatively minor thing, I think there are some issues with over commenting. For example:

/**
     * Returns a random action.
     * 
     * @return  A random action.
     */
    protected A getRandomAction() {
        return actions.get(random.nextInt(actions.size()));
    }

This is a clearly written method, it's method name concisely explains what it does, and there are no parameters to trip up a user. It's got a single line and it's very readable. In fact it takes me longer to read the comment than the code. From Clean Code: a comment should be included if it makes the code easier to understand. Here, we have lots of repetition.

share|improve this answer
3  
Javadoc is not the same as comments –  rolfl Mar 22 at 3:12
1  
Unfortunately, rolfl is right here. Often there is a requirement on code to have appropriate Javadoc, in this case skiwi put that as a requirement on himself apparently. This is still an answer worth thinking about though. But writing non-duplicated Javadoc is hard. –  Simon André Forsberg Mar 22 at 10:33
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.