2
\$\begingroup\$

This is a follow-up question of this one : Implementing a Roll Table data structure

My code at the time was still a bit all over the place, I have since cleaned it up, and I think ended up with a much better one, although it still lacks some functionalities. As a reminder, the goal here is to design a data structure that can hold Dice Roll Tables (the ones you can frequently find in paper RPG games such as Dungeon and Dragons). These tables are defined by the roll to use (for example : throw 2 dices of size 10) and associates the result of that roll to a consequence.

These tables are represented by an interface, RollTable which defines most of the methods I need. The first two are used to fill up the table, whereas the last one is used to roll the actual dice.

public interface RollTable<E> {

    String getName();

    void addRollResult(int minRoll, int maxRoll, E rollResult) throws RollTableInvalidException;

    void linkOtherTable(int minRoll, int maxRoll, RollTable<E> otherTable) throws RollTableInvalidException;

    E rollOnTable() throws RollTableInvalidException;

}

Most of the work is implemented in an abstract class, AbstractRollTable.

public abstract class AbstractRollTable<E> implements RollTable<E> {

    protected static final int DEFAULT_DICE_NUMBER = 1;
    protected static final int DEFAULT_DICE_SIZE = 10;

    private final String name;
    private final int diceSize;
    private final int nbDices;

    private SortedMap<Integer, Callable<E>> rollResultsMap;


    public AbstractRollTable(String name) throws RollTableInvalidException  {
        this(name, DEFAULT_DICE_NUMBER, DEFAULT_DICE_SIZE);
    }

    public AbstractRollTable(String name, int diceSize) throws RollTableInvalidException {
        this(name, DEFAULT_DICE_NUMBER, diceSize);
    }

    public AbstractRollTable(String name, int nbDices, int diceSize) throws RollTableInvalidException {
        if (nbDices <= 0 || diceSize < 1) {
            throw new RollTableInvalidException();
        }
        this.nbDices = nbDices;
        this.diceSize = diceSize;
        this.name = name;
        this.rollResultsMap = new TreeMap<>();
        for (int i : getPossibleRolls()) {
            this.rollResultsMap.put(i, null);
        }
    }

    @Override
    public void addRollResult(int minRoll, int maxRoll, E rollResult) throws RollTableInvalidException {
        int minFixed = minRoll < maxRoll ? minRoll : maxRoll;
        int maxFixed = maxRoll > minRoll ? maxRoll : minRoll;
        checkBounds(minFixed, maxFixed);
        for (int i = minFixed ; i <= maxFixed ; i++) {
            this.rollResultsMap.put(i, getSimpleRollResultCallable(rollResult));
        }
    }

    @Override
    public void linkOtherTable(int minRoll, int maxRoll, RollTable<E> otherRollTable) throws RollTableInvalidException {
        int minFixed = minRoll < maxRoll ? minRoll : maxRoll;
        int maxFixed = maxRoll > minRoll ? maxRoll : minRoll;
        checkBounds(minFixed, maxFixed);
        for (int i = minFixed ; i <= maxFixed ; i++) {
            this.rollResultsMap.put(i, getLinkedRollResultCallable(otherRollTable));
        }
    }

    protected void checkBounds(int minFixed, int maxFixed) throws RollTableInvalidException {
        if (minFixed < rollResultsMap.firstKey() || maxFixed > rollResultsMap.lastKey()) {
            throw new RollTableInvalidException();
        }
    }

    @Override
    public E rollOnTable() throws RollTableInvalidException {
        if (!isRollTableValid()) {
            throw new RollTableInvalidException();
        }
        try {
            return rollResultsMap.get(getRollTableDice()).call();
        } catch (Exception e) {
            throw new RollTableInvalidException(e);
        }
    }

    @Override
    public String toString() {
        StringBuilder builder = new StringBuilder(getName()).append("\n\n");
        for (Entry<Integer, Callable<E>> entry : rollResultsMap.entrySet()) {
            builder.append(String.format("%4s", entry.getKey())).append(" | ").append(entry.getValue()).append('\n');
        }
        return builder.toString();
    }

    @Override
    public String getName() {
        return name;
    }

    protected Map<Integer, Callable<E>> getRollResultsMap() {
        return Collections.unmodifiableMap(rollResultsMap);
    }

    protected int getDiceSize() {
        return diceSize;
    }

    protected int getNbDices() {
        return nbDices;
    }

    protected int getMaxDiceRoll() {
        return getDiceSize() * getNbDices();
    }

    private boolean isRollTableValid() {
        for (Callable<E> rollResult : rollResultsMap.values()) {
            if (rollResult == null) {
                return false;
            }
        }
        return true;
    }

    protected abstract Callable<E> getSimpleRollResultCallable(E rollResult);

    protected abstract Callable<E> getLinkedRollResultCallable(RollTable<E> linkedRollTable);

    protected abstract int[] getPossibleRolls();

    protected abstract int getRollTableDice();

}

As you can see, I use Callable to actually roll on the table. That allows me to link several tables between them (For example, if you get a 7 on Table 1, roll a dice on Table 3). To do that, I use the class RollTableCallable

public class RollTableCallable<E> implements Callable<E> {

    private RollTable<E> linkedRollTable;
    private E rollResult;

    public RollTableCallable(E result) {
        this.rollResult = result;
    }

    public RollTableCallable(RollTable<E> linkedRollTable) {
        this.linkedRollTable = linkedRollTable;
    }

    @Override
    public E call() throws Exception {
        return linkedRollTable != null ? linkedRollTable.rollOnTable() : rollResult;
    }

    @Override
    public String toString() {
        return linkedRollTable != null ? "Roll on " + linkedRollTable.getName() : rollResult.toString();
    }

}

The simplest RollTable is implemented in the class SimpleRollTable, which is pretty basic.

public class SimpleRollTable<E> extends AbstractRollTable<E> {

    public SimpleRollTable(String name) throws RollTableInvalidException  {
        super(name);
    }

    public SimpleRollTable(String name, int diceSize) throws RollTableInvalidException  {
        super(name, diceSize);
    }

    public SimpleRollTable(String name, int nbDices, int diceSize) throws RollTableInvalidException {
        super(name, nbDices, diceSize);
    }

    @Override
    protected int getRollTableDice() {
        int sum = 0;
        for (int i = 0 ; i < getNbDices() ; i++) {
            sum += DiceUtils.roll(getDiceSize());
        }
        return sum;
    }

    @Override
    protected int[] getPossibleRolls() {
        int[] rolls = new int[getNbDices() * getDiceSize() - (getNbDices() - 1)];
        for (int i = 0 ; i < rolls.length ; i++) {
            rolls[i] = i + getNbDices();
        }
        return rolls;
    }

    @Override
    protected Callable<E> getSimpleRollResultCallable(E rollResult) {
        return new RollTableCallable<>(rollResult);
    }

    @Override
    protected Callable<E> getLinkedRollResultCallable(RollTable<E> linkedRollTable) {
        return new RollTableCallable<>(linkedRollTable);
    }

}

A more complex table allows for the use of incremental dice rolls, where one of the consequence of the roll is to give a bonus (or penalty) to the next roll(s). If a roll goes under the minimal value, or above the maximal value, the nearest bound is used instead, and the current increment is reset. This behavior is represented in the interface IncrementalRollTable and implemented in the class IncrementalSimpleRollTable

public interface IncrementalRollTable<E> extends RollTable<E> {

    void addIncrement(int minRoll, int maxRoll, int increment) throws RollTableInvalidException;

}

public class IncrementalSimpleRollTable<E> extends SimpleRollTable<E> implements IncrementalRollTable<E> {

    private int currentIncrement = 0;
    private Map<Integer, Integer> incrementsMap;


    public IncrementalSimpleRollTable(String name) throws RollTableInvalidException {
        this(name, DEFAULT_DICE_NUMBER, DEFAULT_DICE_SIZE);
    }

    public IncrementalSimpleRollTable(String name, int diceSize) throws RollTableInvalidException {
        this(name, DEFAULT_DICE_NUMBER, diceSize);
    }

    public IncrementalSimpleRollTable(String name, int nbDices, int diceSize) throws RollTableInvalidException {
        super(name, nbDices, diceSize);
        this.incrementsMap = new HashMap<>();
        for (int i : getPossibleRolls()) {
            this.incrementsMap.put(i, 0);
        }
    }

    @Override
    public void addIncrement(int minRoll, int maxRoll, int increment) throws RollTableInvalidException {
        int minFixed = minRoll < maxRoll ? minRoll : maxRoll;
        int maxFixed = maxRoll > minRoll ? maxRoll : minRoll;
        checkBounds(minFixed, maxFixed);
        for (int i = minFixed ; i <= maxFixed ; i++) {
            this.incrementsMap.put(i, increment);
        }
    }

    @Override
    protected int getRollTableDice() {
        int diceRoll = super.getRollTableDice();
        int increment = incrementsMap.get(diceRoll);
        diceRoll += currentIncrement;
        if (diceRoll > getMaxDiceRoll()) {
            diceRoll = getMaxDiceRoll();
            currentIncrement = 0;
        } else if (diceRoll < 1) {
            diceRoll = 1;
            currentIncrement = 0;
        } else {
            currentIncrement += increment;
        }
        return diceRoll;
    }

    @Override
    public String toString() {
        StringBuilder builder = new StringBuilder(getName()).append("\n\n");
        for (Entry<Integer, Callable<E>> entry : getRollResultsMap().entrySet()) {
            builder.append(String.format("%4s", entry.getKey())).append(" | ").append(entry.getValue());
            builder.append(" | ").append("Increment : ").append(incrementsMap.get(entry.getKey())).append("\n");
        }
        return builder.toString();
    }

}

The functionalities here are still pretty basic, but I'm pretty sure there is a lot to improve already. I haven't implemented a way to easily build these tables yet (It will probably a JSON parsing, although I'm not sure how to translate the Java generics into that). Thanks a lot !

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

This won't be a complete review, just a collection of thoughts I had while reading your code.


I don't think it's a good idea to limit this to (and build in) a specific kind of dice roll namely dice count * dice size. This has two disadvantages:

  1. You wont be able to model more unusual rolls such as d4+d6 or the use of unusual dice, say FUDGE/FATE dice with range -1/0/1.

  2. More importantly you'll be need dice rolls for other uses other than roll tables, and it would be silly to implement them multiple times.

Also the roll table doesn't need to know all the possible results.

The roll table just needs:

  • possible minimum and maximum roll results

  • a way to get a dice roll


You should have a Range class that represents a minimum and maximum value. That way you don't have to repeat the validation of them in your code and you can extract functionality such as checking overlap, or that one Range lies within another.


You should consider using a builder or factory pattern to generate the roll tables. That way to will be impossible to create an invalid table (or at least the invalidity error will be thrown when the table is built, not when it is used).

\$\endgroup\$
1
  • \$\begingroup\$ I've thought about building a Dice class to replace DiceUtils, I guess this should be the direction to go. For the possible rolls, I've implemented it that way because I thought it would be simpler to manipulate but I'm not 100% satisfied either ... Thanks for your suggestions ! \$\endgroup\$
    – B. Kovac
    Aug 25, 2017 at 8:49

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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