DnD Die in Java with regex

Question

For my game I need to roll some values with a die. A die is formally described as:

nDs[+a]

• n amount of die (optional, 1 of not set, zero must be set explicitly)
• D is a fixed part representing this expression as die
• s amount sides of the die/dice
• +a additional amount

examples:

• 2D6 : 2 six-sided dice, result 2..12
• 2D6+1 : 2 six-sided dice + 1, result 3..13
• 3D4+2 : 3 four-sided dice + 2, result 5..14

Implementation of class Die

public class Die {

    private static final String INPUT_VALIDATION_PATTERN = "^\\d*D\\d*(\\+\\d+)?$";
    private static final String SINGLE_DIE_PATTERN = "\\d*D\\d*";
    private static final String ADDITION_SEPARATOR_PATTERN = "\\+";
    private static final String DIE_SEPARATOR_PATTERN = "D";
    private static final String ADD_PATTERN = "\\d*";
    private static final String WHITESPACE_PATTERN = "\\s+";

    //package visible for testing
    final int amount;
    final int sides;
    final int addition;
    
    public Die(String input) {
        String adjusted = input.toUpperCase(Locale.ROOT).replaceAll(WHITESPACE_PATTERN,"");
        if (!adjusted.matches(INPUT_VALIDATION_PATTERN)) {
            throw new IllegalArgumentException(
                "invalid dice input pattern: "+adjusted+" - does not match "+INPUT_VALIDATION_PATTERN);
        }
        
        String[] parts = input.split(ADDITION_SEPARATOR_PATTERN);
        if(parts[0].matches(SINGLE_DIE_PATTERN)){
            String dicePart = parts[0];
            int dIndex = dicePart.indexOf(DIE_SEPARATOR_PATTERN);
            if(dIndex == 0){
                amount = 1;
            }else{
                amount = Integer.parseInt(dicePart.substring(0,dIndex));
            }
            sides = Integer.parseInt(dicePart.substring(dIndex+1));
        }else{
            amount = 0;
            sides = 0;
        }

        if(parts[0].matches(ADD_PATTERN)){
            addition = Integer.parseInt(parts[0]);
        }else if(parts.length == 2 && parts[1].matches(ADD_PATTERN)){
            addition = Integer.parseInt(parts[1]);
        }else{
            addition = 0;
        }
    }
    
    public int result(long seed){
        Random random = new Random(seed);
        int result = 0;
        for(int roll = 0; roll < amount; roll++){
            result = result + random.nextInt(sides)+1;
        }
        result = result + addition;
        return result;
    }

    public int result(){
        return result(0);
    }

    public int min(){
        return amount + addition;
    }

    public int max(){
        return amount * sides + addition;
    }
}

Test Case DieTest

public class DieTest {

    @Test
    public void singleDieTest(){
        Die d6 = new Die("D6");
        Assert.assertEquals(6, d6.sides);
        Assert.assertEquals(1, d6.amount);
        Assert.assertEquals(0,d6.addition);
        Assert.assertEquals(1,d6.min());
        Assert.assertEquals(6,d6.max());

        Die oneD6 = new Die("1D6");
        Assert.assertEquals(6, oneD6.sides);
        Assert.assertEquals(1, oneD6.amount);
        Assert.assertEquals(0,oneD6.addition);
        Assert.assertEquals(1,oneD6.min());
        Assert.assertEquals(6,oneD6.max());

        Die twoD6 = new Die("2D6");
        Assert.assertEquals(6, twoD6.sides);
        Assert.assertEquals(2, twoD6.amount);
        Assert.assertEquals(0,twoD6.addition);
        Assert.assertEquals(2,twoD6.min());
        Assert.assertEquals(12,twoD6.max());

        Die twentyD6 = new Die("20D6");
        Assert.assertEquals(6, twentyD6.sides);
        Assert.assertEquals(20, twentyD6.amount);
        Assert.assertEquals(0,twentyD6.addition);
        Assert.assertEquals(20,twentyD6.min());
        Assert.assertEquals(120,twentyD6.max());

        Die twentyD60 = new Die("20D60");
        Assert.assertEquals(60, twentyD60.sides);
        Assert.assertEquals(20, twentyD60.amount);
        Assert.assertEquals(0,twentyD60.addition);
        Assert.assertEquals(20,twentyD60.min());
        Assert.assertEquals(1200,twentyD60.max());
    }

    @Test
    public void dieWithSingleAdditionTest(){
        Die d6 = new Die("D6+1");
        Assert.assertEquals(6, d6.sides);
        Assert.assertEquals(1, d6.amount);
        Assert.assertEquals(1,d6.addition);
        Assert.assertEquals(2,d6.min());
        Assert.assertEquals(7,d6.max());

        Die oneD6 = new Die("1D6+1");
        Assert.assertEquals(6, oneD6.sides);
        Assert.assertEquals(1, oneD6.amount);
        Assert.assertEquals(1,oneD6.addition);
        Assert.assertEquals(2,oneD6.min());
        Assert.assertEquals(7,oneD6.max());

        Die twoD6 = new Die("2D6+1");
        Assert.assertEquals(6, twoD6.sides);
        Assert.assertEquals(2, twoD6.amount);
        Assert.assertEquals(1,twoD6.addition);
        Assert.assertEquals(3,twoD6.min());
        Assert.assertEquals(13,twoD6.max());

        Die twentyD6 = new Die("20D6+1");
        Assert.assertEquals(6, twentyD6.sides);
        Assert.assertEquals(20, twentyD6.amount);
        Assert.assertEquals(1,twentyD6.addition);
        Assert.assertEquals(21,twentyD6.min());
        Assert.assertEquals(121,twentyD6.max());

        Die twentyD60 = new Die("20D60+1");
        Assert.assertEquals(60, twentyD60.sides);
        Assert.assertEquals(20, twentyD60.amount);
        Assert.assertEquals(1,twentyD60.addition);
        Assert.assertEquals(21,twentyD60.min());
        Assert.assertEquals(1201,twentyD60.max());
    }

    @Test
    public void dieWithTwoDigitAdditionTest(){
        Die d6 = new Die("D6+13");
        Assert.assertEquals(6, d6.sides);
        Assert.assertEquals(1, d6.amount);
        Assert.assertEquals(13,d6.addition);
        Assert.assertEquals(14,d6.min());
        Assert.assertEquals(19,d6.max());

        Die oneD6 = new Die("1D6+13");
        Assert.assertEquals(6, oneD6.sides);
        Assert.assertEquals(1, oneD6.amount);
        Assert.assertEquals(13,oneD6.addition);
        Assert.assertEquals(14,oneD6.min());
        Assert.assertEquals(19,oneD6.max());

        Die twoD6 = new Die("2D6+13");
        Assert.assertEquals(6, twoD6.sides);
        Assert.assertEquals(2, twoD6.amount);
        Assert.assertEquals(13,twoD6.addition);
        Assert.assertEquals(15,twoD6.min());
        Assert.assertEquals(25,twoD6.max());

        Die twentyD6 = new Die("20D6+13");
        Assert.assertEquals(6, twentyD6.sides);
        Assert.assertEquals(20, twentyD6.amount);
        Assert.assertEquals(13,twentyD6.addition);
        Assert.assertEquals(33,twentyD6.min());
        Assert.assertEquals(133,twentyD6.max());

        Die twentyD60 = new Die("20D60+13");
        Assert.assertEquals(60, twentyD60.sides);
        Assert.assertEquals(20, twentyD60.amount);
        Assert.assertEquals(13,twentyD60.addition);
        Assert.assertEquals(33,twentyD60.min());
        Assert.assertEquals(1213,twentyD60.max());
    }

    @Test
    public void rangeTest(){
        Die twoD6plus1 = new Die("2D6+1");
        for (int i = 0; i < 100; i++){
            Assert.assertTrue(twoD6plus1.result() >= twoD6plus1.min()  );
            Assert.assertTrue(twoD6plus1.result() <= twoD6plus1.max()   );
        }
    }
}

Answer 1

Bug in the validation pattern

The INPUT_VALIDATION_PATTERN doesn't work correctly, as it matches the invalid input "D". This is due to the pattern looking for any number of digits (*) after the D. Changing the pattern to look for at least one occurrence (+) seems to fix this.

Simple solution using capture groups

After some fiddling around with your pattern I found that regex can be used to solve this rather nicely, using ^(\d+)?(D\d+)(\+\d+)?$.

String pattern = "^(\\d+)?(D\\d+)(\\+\\d+)?$";
Pattern r = Pattern.compile(pattern);
Matcher m = r.matcher(in);

if (m.find()) {
    ...
}

If the input matches, m.find() returns true. You then use m.group(x) to access the capture results, with m.group(1) being the number of dice, m.group(2) being the number of sides and m.group(3) being the addition. If there is no number for multiple dice/addition, then the groups 1/3 return null, which can easily be tested for. Do note that the strings in group 2 and 3 start with D and + respectively due to how the groups are set up. How the full constructor would look is left as an exercise for the reader :)

Explaination:

^(\d+)?(D\d+)(\+\d+)?$
^                    $  // start/end of string
 (   )                  // capture group 1: capture
      ?                 // one or zero of
   \d+                  // at least one digit
       (    )           // capture group 2: capture
        D\d+            // a "D" followed by at least one digit
             (     )    // capture group 3: capture
                    ?   // one or zero of
              \+\d+     // a "+" followed by at least one digit

I'm no expert on regex, so please test this thoroughly. I've thrown your tests, some edge cases and random things at this regex and it worked, but you can't be too careful.

Answer 2

We don't know how your Die fits into the game as a whole, but the way you're rolling your results seems counter-intuitive to me. I'd expect to be able to call result and get a unique value each time. However, if I roll 5D6+2, I'll always get the same answer (22 on my system), because it's always creating a new Random with a known seed (0). Consider moving Random outside of the result method, or having a different value for the default seed (such as something time-based), or separating the rolling responsibility out into a different class entirely.

public int result(long seed){
    Random random = new Random(seed);
    int result = 0;
    for(int roll = 0; roll < amount; roll++){
        result = result + random.nextInt(sides)+1;
    }
    result = result + addition;
    return result;
}

public int result(){
    return result(0);
}

As a side note... should your class be called Dice? It seems to represent more than one Die? amount could then perhaps be numberOfDie..

Answer 3

You can use named groups in the pattern, as well, which will make the code less brittle to pattern changes. (Example: If you want to add in the feature of multiplication in the future then you could have, e.g., 3d6x2+1 meaning that the addition part would be the 4th matching group instead of the 3rd, and if you refer to groups by order you would have to update the addition part of your code. If you refer to groups by name, then the addition group is still the addition group regardless of where it moves to, so you would only have to touch the new code.) It can also make counting groups in general a lot easier.

String diceNotation = "(?<amount>\\d+)?D(?<faces>\\d+)(\\+(?<addition>\\d))?";
Pattern dicePattern = Pattern.compile(diceNotation, Pattern.CASE_INSENSITIVE);
Matcher diceMatcher = dicePattern.matcher(input);

int amount = diceMatcher.group("amount") != null ? 
    Integer.parseInt(diceMatcher.group("amount")) :
    1;
int faces = Integer.parseInt(diceMatcher.group("faces"));
int add = diceMatcher.group("add") != null ?
    Integer.parseInt(diceMatcher.group("add")) :
    0;

Note that Matcher.group is returning null when a pattern doesn't match at all. There are some subtleties of regex here, such as (?<foo>a*) will match an empty string and return an empty string as the foo group, but (?<foo>a+)? will match an empty string with the whole pattern but return null as the foo group.

Also, I have taken the liberty of including case_insensitive, because 3d6 is just as good as 3D6. You could also liberally sprinkle \s* between each group in order to allow arbitrary white space, so that 3 d 6 = 3d 6 = 3d6 = etc...

Answer 4

As others have mentioned, there is a bug in the pattern - see their answers for assistance on fixing it ;)

Apart from this, I'd like to remind you to refactor your tests as well. The repeated pattern in the tests, along with the relatively large amount of boilerplate, make it hard to distinguish if there are any cases you have missed or if there are duplicates.

There are solutions out there for helping to write tests that focus on what you are actually trying to test, instead of writing pages full of asserts. For instance, https://www.baeldung.com/parameterized-tests-junit-5 :

private static Stream<Arguments> shouldParseDice() {
    return Stream.of(
      Arguments.of("D6", 1, 6, 0, 1, 6),
      Arguments.of("1D6", 1, 6, 0, 1, 6),
      Arguments.of("2D6", 2, 6, 0, 2, 12),
      Arguments.of("2D6+1", 2, 6, 1, 3, 13),
// ...
      Arguments.of("30D11+40", 30, 11, 40, 70, 370)
    );
}

@ParameterizedTest
@MethodSource 
void shouldParseDice(
  String input, 
  Integer amount,
  Integer sides,
  Integer addition,
  Integer min,
  Integer max) {
        Die die = new Die(input);
        Assertions.assertEquals(amount, die.amount);
        Assertions.assertEquals(sides, die.sides);
        Assertions.assertEquals(addition, die.addition);
        Assertions.assertEquals(min, die.min());
        Assertions.assertEquals(max, die.max());
}

private static Stream<String> shouldFailDieParsing() {
    return Stream.of("D", "G", "2D6-4", "", null, "-23D-2", "-2D1");
}

@ParameterizedTest
@MethodSource 
void shouldFailDieParsing(String input) {
    
    Assertions.assertThrows<IllegalArgumentException>(() => new Die(input));
}

If, for whatever reason, you are unable to make the move to junit5, you could simply roll your own in junit4, though it is slightly less tool supported and need some more explicit details to be able to easily understand the output when it fails:

@Test
public void shouldParseDice() {
    validateDieParsing("D6", 1, 6, 0, 1, 6);
    validateDieParsing("1D6", 1, 6, 0, 1, 6);
    validateDieParsing("2D6", 2, 6, 0, 2, 12);
    validateDieParsing("2D6+1", 2, 6, 1, 3, 13);
// ...
    validateDieParsing("30D11+40", 30, 11, 40, 70, 370);
}

private void validateDieParsing(
  String input, 
  Integer amount,
  Integer sides,
  Integer addition,
  Integer min,
  Integer max) {
        Die die = new Die(input);
        String message = "input was: " + input;
        Assert.assertEquals(message, amount, die.amount);
        Assert.assertEquals(message, sides, die.sides);
        Assert.assertEquals(message, addition, die.addition);
        Assert.assertEquals(message, min, die.min());
        Assert.assertEquals(message, max, die.max());
}

@Test
public void shouldFailDieParsing() {
    return Stream.of("D", "G", "2D6-4", "", null, "-23D-2", "-2D1")
      .forEach(it -> shouldFailDieParsing(it));
}

private void shouldFailDieParsing(String input) {
    try {
       new Die(input);
       fail("did not throw exception on expected bad input = " + input);    
    } catch (IllegalArgumentException e) {
       // expected, ignore
    } catch (Exception e) {
      e.printStackTrace():
      fail("Caught unexpected exception for input = " + input);
    }
}

Transforming your tests to a format where you can easily see which cases you have covered, makes it easier to write good tests. In this case, you may have discovered the "D" input giving you trouble early on.

Answer 5

From the design standpoint it seems you are mixing two things - abstraction of a die (that has certain properties like number of faces) and has some behavior associated with it (as it can be rolled for certain number to be a result) AND parsing (even though you call it differently see: https://lexi-lambda.github.io/blog/2019/11/05/parse-don-t-validate/) seemingly arbitrary string.

I don't know application of this class - which, imho, should be major driver the design of a class, but you might consider separating those two - especially if creating dice set from a raw string is somehow accidental (so if you ever have a need to roll dice but can do it without parsing strings - my guess would be for something like standard monsters having specific attack dices).

Given bugs in parsing pointed out in other comments - you should probably consider if another implementation approach would allow you to avoid them by its nature. One of the problem sources was use of regex in parsing (I am not great fan of it in general) - are you sure you need regex (or in that amount)? Maybe you can just "text".indexOf("D") and split the string by your local grammar in parts - writing simple parsers is really not that hard. There is also another issue with regexes - if your class is anytime to parse untrusted input in current form it could be relatively easy to it make it use a lot of resources (see e.g. https://s0md3v.medium.com/exploiting-regular-expressions-2192dbbd6936) - although it could be simply circumvented by introducing some input length limit.

Another thing how you test randomness of dices (and how it didn't catch the bug with new Random(0)) - notice that you have two methods for result in dice and only one of them is used in tests (the other seems unused but I guess that it might used somewhere else), usually this situation is a manifestation of a design flaw and it is a way for tests to tell us something valuable (pay attention to those) - here it is a way of saying that randomness seed is an external dependency and it is important enough to be identified and treated with respect.
Usually extracting seed and passing it as a dependency (in whatever way you manage them) is enough to write deterministic tests that check actual results in such case. I'll repeat it because it is so important: test for results that you care about, not what is easy to test at the moment (like testing that d4 roll can result in 1,2,3,4 values and not that result is between min and max - with the right seed you shouldn't need hundreds of attempts) and don't hesitate to change the design so that it is easy to test.

These are my main points, some related ideas that sparked in my head when seeing this code:

• die vs dice looks like a nice place for composite pattern (I wouldn't force it, simple is better than using arbitrary pattern - but if needed it this design could be leaned into)
• if you wanted you could test distribution of results (a little over the top, I am aware, but if it was part of some safety critical software... then don't use dices :D)
• you could use https://github.com/VerbalExpressions/JavaVerbalExpressions (I don't think any use of regex is small enough, to not use it but its only my personal opinion) but better not use it at all
• Seeing Greg Burghardt comment I, personally don't fully agree with constructors doing work being a bad thing (it surely can be bad) - I think that static parsing/validation (checking invariants) can be placed in constructor as a way to strengthen compile-time(ish) type safety and (again IMO) the root problem here is (mentioned at the beginning) mixture of ideas of parsing and rollable dice

Answer 6

About the difference between Perl and Java

If this class will be used a lot, there are a couple other things you should probably do:

1. Compile the patterns. Don't use the convenience methods in String. Use java.util.regex.Pattern explicitly and cache it in a static. Thus:

   private final static Pattern INPUT_VALIDATION_PATTERN = Pattern.compile(....);  
   

   (This was shown in @mindoverflow's answer, as it is also what is needed to use groups.) The real point here is that pattern compiling is costly, so you should only do it once, if you can. Perl will do this automatically if the pattern is a constant, and has a flag to force it if the pattern is variable. In Java, one does this oneself.
2. Java's regular expression support is actually more complete than Perl's. It has matches() (and you are using it). Thus, your pattern doesn't need a leading ^ and trailing $, as these are implied by the function. Note that matches() will generally be faster than find(), as the later tends to do some costly optimizations to speed up the search.

(Other people have addressed the general pattern changes, so I won't.)

Answer 7

I had answered the question, but my previous answer contained some factual errors. Thanks to chepner, Eric Stein and JDługosz for the corrections, and additional info about Narrative Dice which have symbols on the die faces instead of numbers.

---

You have plenty of answers about regular expressions. Instead, I will focus on the overall object oriented design.

The word "Die" is the singular form of "Dice". It was surprising to me that a "Die" object represented more than one item. Consider separating the concepts of a "die" from the algorithm used to determine a dice roll, and the result of that roll. This implies two separate classes: DiceRollGenerator and DiceRoll. For the purposes of this question I will assume all die face values are integers, but you could use Generic Types in Java on the DiceRollGenerator class to support additional die face types.

The current Die constructor is responsible for parsing the dice roll format, which means the constructor is doing real work. Constructors should not do real work. Instead, the constructor should do little more than set values on fields. In the case of the DiceRollGenerator, this would be the number of dice, the number of faces for each die, and the additional amount added to each dice roll.

int numberOfDice = 4;
int sides = 6;
int amount = 2;
DiceRollGenerator generator = new DiceRollGenerator(numberOfDice, sides, amount);

Still, it is desirable to parse a well-known dice format string. Parsing logic is error-prone. When parsing, both the parser and the caller should expect errors. This is not ideal for a constructor. Parsing logic should be moved into a static method on the class with the most knowledge about how to generate a dice roll: the DiceRollGenerator.

DiceRollGenerator generator = DiceRollGenerator.parse("4D6+2");

The parse method should throw exceptions given invalid input. The constructor, with its simpler input, has fewer reasons to throw an exception, making the constructor easier to call. The parse method is inherently harder to call due to the myriad reasons for it to fail. This allows callers to choose which technique fits best for the use case.

You can eliminate the clutter of exception handling by having the parse method return an Optional<DiceRollGenerator> object instead of throwing exceptions when given invalid input. Just be sure to document what it means when an empty Optional<DiceRollGenerator> is returned using JavaDoc comments. An empty "optional" implies the dice roll format was incorrect.

Creating the first dice roll would be a simple call to generator.rollDice(). This method would generate a random number as the seed for the dice roll. It would return a DiceRoll object.

DiceRoll roll = generator.rollDice();

The DiceRoll class should expose methods that allow you to interrogate the current state of the dice roll. This allows you to build additional game rules on top of the dice roll logic:

int firstDieValue = roll.getDieValue(0); // returns the value of the die object at index 0
int total = roll.getTotal();
boolean didDoubleSixesGetRolled = roll.hasDoubles(6);

Subsequent dice rolls also need a seed value. You can generate a new random number, or use an existing dice roll to generate the new seed:

roll = generator.rollDice(roll);

This could help simulate a real physical dice roll, since the player picks up the existing dice to roll them again. This, of course, is not necessary for a computer game, and simply generating a random roll each time could suffice.

To summarize:

• Introduce a DiceRoll class to encapsulate the result of a dice roll.
• Rename the Die class to something more descriptive of what it does, like DiceRollGenerator
• Move parsing logic into a static method, which simplifies object construction in use cases where parsing the dice format is not necessary.