5
\$\begingroup\$

In this android app, the user can enter (a subset of) roll20 notation to roll that many dice. Inputs will look like "4d6" at the simplest, but the user can also add or subtract numbers, and drop or keep the highest or lowest dice. I have implemented a class to do this parsing, with public methods for the resultant numbers.

Roll20NotationString.java

package com.mwapp.ron.fancydice;

import java.util.regex.Matcher;
import java.util.regex.Pattern;

/**
 * Helper class to parse roll20 notation.
 */
public class Roll20NotationString {
    private int numDice;
    private int numSides;
    private int modifier;
    private boolean dropWasSpecified;
    private int dropLow;
    private int dropHigh;

    //Regex usage example on next line, aligned with relevant parts of the regex; rolls 4d6, drop lowest, +1.  Some of these are optional.  _ marks allowable whitespace
    //"4d6dl1 + 1" ->                                    _   4   d  6             dl         1     _      +     _   1       _
    private static final String GRAND_ROLL20_REGEX = "^\\s*(\\d+)d(\\d+)((d|k|dl|kh|dh|kl)(\\d*))?\\s*((\\+|-)\\s*(\\d+))?\\s*$";
    private static final int NUMDICE_CAPTURE_GROUP = 1;
    private static final int NUMSIDES_CAPTURE_GROUP = 2;
    //Capture group 3 will be "dl1."  Not useful.
    private static final int DROP_KEEP_MODE_CAPTURE_GROUP = 4; //Nullable
    private static final int DROP_KEEP_NUMBER_CAPTURE_GROUP = 5; //Nullable
    //Capture group 6 will be "+ 1"
    private static final int PLUS_MINUS_CAPTURE_GROUP = 7; //Nullable
    private static final int PLUS_MINUS_VALUE_CAPTURE_GROUP = 8; //Nullable

    private static final Pattern grandRoll20RegexPattern = Pattern.compile(GRAND_ROLL20_REGEX);

    public Roll20NotationString(String source) throws InvalidNotationException {
        parseString(source);
    }

    private void parseString(String source) throws InvalidNotationException {
        Matcher matcher = grandRoll20RegexPattern.matcher(source);
        if (!matcher.matches()) throw new InvalidNotationException();
        parseCoreStuff(matcher);
        parseDropKeep(matcher);
        parseModifier(matcher);
    }

    private void parseCoreStuff(Matcher matcher) {
        numDice = Integer.parseInt(matcher.group(NUMDICE_CAPTURE_GROUP));
        numSides = Integer.parseInt(matcher.group(NUMSIDES_CAPTURE_GROUP));
    }

    private void parseDropKeep(Matcher matcher) {
        dropLow = dropHigh = 0; //In case the groups are null.
        String dropKeepNumberStr = matcher.group(DROP_KEEP_NUMBER_CAPTURE_GROUP);
        int dropKeepNumber = (dropKeepNumberStr == null || dropKeepNumberStr.isEmpty()) ? 1 : Integer.parseInt(dropKeepNumberStr);
        String dropKeepMode = matcher.group(DROP_KEEP_MODE_CAPTURE_GROUP);
        if (dropKeepMode != null) {
            dropWasSpecified = true;
            switch (dropKeepMode) {
                case "d": //Drop lowest
                case "dl":
                    dropLow = dropKeepNumber;
                    break;
                case "k": //Keep highest
                case "kh":
                    dropLow = numDice - dropKeepNumber;
                    break;
                case "dh": //Drop highest
                    dropHigh = dropKeepNumber;
                    break;
                case "kl": //Keep lowest
                    dropHigh = numDice - dropKeepNumber;
                    break;
            }
        } else {
            dropWasSpecified = false;
        }
    }

    private void parseModifier(Matcher matcher) {
        String plusOrMinus = matcher.group(PLUS_MINUS_CAPTURE_GROUP);
        if (plusOrMinus == null) {
            modifier = 0;
            return;
        }
        int plusMinusOne = plusOrMinus.equals("-") ? -1 : 1;
        modifier = Integer.parseInt(matcher.group(PLUS_MINUS_VALUE_CAPTURE_GROUP)) * plusMinusOne;
    }

    public int getNumDice() {
        return numDice;
    }

    public int getNumSides() {
        return numSides;
    }

    public int getModifier() {
        return modifier;
    }

    public boolean dropWasSpecified() {
        return dropWasSpecified;
    }

    public int getDropLow() {
        return dropLow;
    }

    public int getDropHigh() {
        return dropHigh;
    }

    public class InvalidNotationException extends Exception { //Checked.  Because it's probably gonna happen.
        public InvalidNotationException() { super(); }
        public InvalidNotationException(String message) { super(message); }
        public InvalidNotationException(String message, Throwable cause) { super(message, cause); }
        public InvalidNotationException(Throwable cause) { super(cause); }
    }
}
\$\endgroup\$
1
\$\begingroup\$

Regex

The regex should be constructed in a way that makes it self-documenting. In particular, you should use named capture groups. Also, you can write the string literal over multiple lines, with // comments at the end of each line if needed.

When using parentheses for grouping but not capturing, write (?:stuff) rather than (stuff).

In my opinion, whitespace should be allowed before the drop/keep mode.

Class design

The InvalidNotationException inner class should be static. You aren't using three of its constructors, so you can omit all four constructors for now.

The parseString() helper method should just be written directly in the constructor.

I suggest validating that the number of dice being dropped does not exceed the number of dice rolled.

A toString() method, returning a canonical representation, would be helpful for future debugging.

Suggested solution

import java.util.regex.Matcher;
import java.util.regex.Pattern;

public class Roll20NotationString {
    public static class InvalidNotationException extends Exception {
        //Checked.  Because it's probably gonna happen.
    }

    private int numDice;
    private int numSides;
    private int modifier;
    private boolean dropWasSpecified;
    private int dropLow;
    private int dropHigh;

    private static final Pattern ROLL20_REGEX = Pattern.compile(
        "^\\s*" +
        "(?<numDice>\\d+)d(?<numSides>\\d+)\\s*" +
        "(?:(?<dropKeepMode>d|k|dl|kh|dh|kl)(?<dropKeepNum>\\d*))?\\s*" +
        "(?:(?<plusMinus>[+-])\\s*(?<plusMinusValue>\\d+))?" +
        "\\s*$"
    );

    public Roll20NotationString(String source) throws InvalidNotationException {
        Matcher matcher = ROLL20_REGEX.matcher(source);
        if (!matcher.matches()) throw new InvalidNotationException();
        parseCoreStuff(matcher);
        parseDropKeep(matcher);
        parseModifier(matcher);
    }

    private void parseCoreStuff(Matcher matcher) {
        numDice = Integer.parseInt(matcher.group("numDice"));
        numSides = Integer.parseInt(matcher.group("numSides"));
    }

    private void parseDropKeep(Matcher matcher) throws InvalidNotationException {
        if (matcher.group("dropKeepMode") == null) {
            return;
        }
        dropWasSpecified = true;

        String dropKeepNumberStr = matcher.group("dropKeepNum");
        int dropKeepNumber = dropKeepNumberStr.isEmpty() ?
            1 : Integer.parseInt(dropKeepNumberStr);
        switch (matcher.group("dropKeepMode")) {
          case "d": //Drop lowest
          case "dl":
            dropLow = dropKeepNumber;
            break;
          case "k": //Keep highest
          case "kh":
            dropLow = numDice - dropKeepNumber;
            break;
          case "dh": //Drop highest
            dropHigh = dropKeepNumber;
            break;
          case "kl": //Keep lowest
            dropHigh = numDice - dropKeepNumber;
            break;
        }
        if (dropLow > numDice || dropHigh > numDice) {
            throw new InvalidNotationException();
        }
        dropWasSpecified = true;
    }

    private void parseModifier(Matcher matcher) {
        modifier = (matcher.group("plusMinus") == null) ?
            0 :
            Integer.parseInt(
                matcher.group("plusMinus") + matcher.group("plusMinusValue")
            );
    }

    public int getNumDice() {
        return numDice;
    }

    public int getNumSides() {
        return numSides;
    }

    public int getModifier() {
        return modifier;
    }

    public boolean dropWasSpecified() {
        return dropWasSpecified;
    }

    public int getDropLow() {
        return dropLow;
    }

    public int getDropHigh() {
        return dropHigh;
    }

    public String toString() {
        return String.format("%dd%d%s%s",
            numDice,
            numSides,
            ((dropLow > 0)       ? String.format(" dl%d", dropLow) :
             (dropHigh > 0)      ? String.format(" dh%d", dropHigh) :
                                   ""),
            ((modifier == 0) ? "" : String.format(" %+d", modifier))
        );
    }

}
\$\endgroup\$
0
\$\begingroup\$

Minor simplifications for the regex:

(d|k|dl|kh|dh|kl)

can be:

[dk][hl]?

Which makes it a bit easier to see what this does.


(\\+|-)

is just

[+-]

This has the additional benefit of not needing to escape the +


Let me also echo what 200_success already stated about named capture-groups.

I personally prefer to not expose constructors that throw Exceptions. If something requires prevalidation, I prefer to expose a static factory method:

public static Roll20NotationString roll(String rollString) throws InvalidNotationException {
    Matcher matcher = grandRoll20RegexPattern.matcher(rollString);
    if (!matcher.matches()) throw new InvalidNotationException();
    // ...
}

Note that this allows you to drop the exception in favor of a "null object", in case you prefer that. This avoids the expensive creation of an exception and trades it for a nullcheck.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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