trjhtr

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 dh

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 k

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.