Update

3 files changed, 123 insertions, 66 deletions

diff --git a/dice-lang/src/bjc/dicelang/dice/CompoundDie.java b/dice-lang/src/bjc/dicelang/dice/CompoundDie.java
index a81322c..0793872 100644
--- a/dice-lang/src/bjc/dicelang/dice/CompoundDie.java
+++ b/dice-lang/src/bjc/dicelang/dice/CompoundDie.java
@@ -6,11 +6,9 @@ package bjc.dicelang.dice;
  * @author Ben Culkin

  */

 public class CompoundDie implements Die {

-	/*

-	 * The dice that form this die

-	 */

-	private final Die       left;

-	private final Die       right;

+	/* The dice that form this die */

+	private final Die left;

+	private final Die right;

 

 	/**

 	 * Create a new compound die.

@@ -32,24 +30,31 @@ public class CompoundDie implements Die {
 

 	@Override

 	public long optimize() {

-		return Long.parseLong(left.optimize() + "" + right.optimize());

+		long leftOpt  = left.optimize();

+		long rightOpt = right.optimize();

+

+		return Long.parseLong(String.format("%d%d", leftOpt, rightOpt));

 	}

 

 	@Override

 	public long roll() {

-		return Long.parseLong(left.roll() + "" + right.roll());

+		long leftRoll  = left.optimize();

+		long rightRoll = right.optimize();

+

+		return Long.parseLong(String.format("%d%d", leftRoll, rightRoll));

 	}

 

 	@Override

 	public long rollSingle() {

-		/*

-		 * We're only one die, we can't be split

-		 */

+		/* Actually one dice built using two, can't be split. */

 		return roll();

 	}

 

 	@Override

 	public String toString() {

-		return left.toString() + "c" + right.toString();

+		String leftString  = left.toString();

+		String rightString = right.toString();

+

+		return String.format("%sc%s", leftString, rightString);

 	}

 }

diff --git a/dice-lang/src/bjc/dicelang/dice/CompoundingDie.java b/dice-lang/src/bjc/dicelang/dice/CompoundingDie.java
index 1bd478f..4b3f137 100644
--- a/dice-lang/src/bjc/dicelang/dice/CompoundingDie.java
+++ b/dice-lang/src/bjc/dicelang/dice/CompoundingDie.java
@@ -11,9 +11,12 @@ import java.util.function.Predicate;
  * @author Ben Culkin

  */

 public class CompoundingDie implements Die {

+	/* The source die to compound. */

 	private final Die source;

 

+	/* The predicate that marks when to compound. */

 	private final Predicate<Long>   compoundOn;

+	/* The string version of the predicate, if one exists. */

 	private final String            compoundPattern;

 

 	/**

@@ -48,22 +51,32 @@ public class CompoundingDie implements Die {
 

 	@Override

 	public boolean canOptimize() {

-		return source.canOptimize() && source.optimize() == 0;

+		if(source.canOptimize()) {

+			/* We can only be optimized for a result of zero. */

+			return source.optimize() == 0;

+		}

+

+		return false;

 	}

 

 	@Override

 	public long optimize() {

+		/* If we can be optimized, its to zero. */

 		return 0;

 	}

 

 	@Override

 	public long roll() {

+		/* The current result. */

 		long res = source.roll();

+		/* The last result. */

 		long oldRes = res;

 

 		while (compoundOn.test(oldRes)) {

+			/* Compound while the result should be compounded. */

 			oldRes = source.rollSingle();

 

+			/* Accumulate. */

 			res += oldRes;

 		}

 

@@ -72,15 +85,16 @@ public class CompoundingDie implements Die {
 

 	@Override

 	public long rollSingle() {

-		/*

-		 * Just compound on a single roll

-		 */

+		/* Just compound on an initial single role. */

 		long res = source.rollSingle();

+		/* The last result. */

 		long oldRes = res;

 

 		while (compoundOn.test(oldRes)) {

+			/* Compound while the result should be compounded. */

 			oldRes = source.rollSingle();

 

+			/* Accumulate. */

 			res += oldRes;

 		}

 

@@ -89,8 +103,13 @@ public class CompoundingDie implements Die {
 

 	@Override

 	public String toString() {

-		if (compoundPattern == null) return source + "!!";

+		String sourceString = source.toString();

+

+		/* Can't print a parseable version. */

+		if (compoundPattern == null) {

+			return String.format("%s!!<complex-pattern>", sourceString);

+		}

 

-		return source + "!!" + compoundPattern;

+		return String.format("%s!!%s", sourceString, compoundPattern);

 	}

 }

diff --git a/dice-lang/src/bjc/dicelang/dice/DiceBox.java b/dice-lang/src/bjc/dicelang/dice/DiceBox.java
index afb2ea3..41b31af 100644
--- a/dice-lang/src/bjc/dicelang/dice/DiceBox.java
+++ b/dice-lang/src/bjc/dicelang/dice/DiceBox.java
@@ -21,110 +21,143 @@ public class DiceBox {
 	 * @return The die expression from the string, or null if it wasn't one
 	 */
 	public static DieExpression parseExpression(final String expString) {
-		/*
-		 * Only bother with valid expressions.
-		 */
-		if (!isValidExpression(expString)) return null;
+		try {
+			return doParseExpression(expString);
+		} catch (Exception ex) {
+			/* 
+			 * @TODO 10/08/17 Ben Culkin :DieErrors :ErrorRefactor
+			 * 	Use different types of exceptions to provide
+			 * 	better error messages. */
+			System.out.println("ERROR: Could not parse die expression (Cause: %s)\n", ex.getMessage);
+			ex.printStackTrace();
+
+			return null;
+		}
+	}
+	
+	private static DieExpression doParseExpression(final String expString) {
+		/* Only bother with valid expressions. */
+		if (!isValidExpression(expString)) {
+			return null;
+		}
 
 		if (scalarDiePattern.matcher(expString).matches()) {
-			/*
-			 * Parse scalar die.
+			/* Parse scalar die. */
+			/* @TODO 10/08/17 Ben Culkin :SubstringIndexOf
+			 * 	This substring/index of call should be
+			 * 	abstracted into its own method so as to make the
+			 * 	code more explanatory and ensure that things
+			 * 	like the return code of indexOf are correctly
+			 * 	checked.
 			 */
 			final String dieString = expString.substring(0, expString.indexOf('s'));
-			final Die scal = new ScalarDie(Long.parseLong(dieString));
+
+			final long lar = Long.parseLong(dieString);
+
+			final Die scal = new ScalarDie(lar);
 
 			return new DieExpression(scal);
 		} else if (simpleDiePattern.matcher(expString).matches()) {
-			/*
-			 * Parse simple die groups.
-			 */
+			/* Parse simple die groups. */
 			final String[] dieParts = expString.split("d");
 
 			final long right = Long.parseLong(dieParts[1]);
+			final long left;
 
 			if (dieParts[0].equals("")) {
-				/*
-				 * Handle short-form expressions.
-				 */
-				final Die scal = new SimpleDie(1, right);
-				return new DieExpression(scal);
+				/* Handle short-form expressions. */
+				left = 1;
+			} else {
+				left = Long.parseLong(dieParts[0]);
 			}
 
-			final Die scal = new SimpleDie(Long.parseLong(dieParts[0]), right);
+			final Die scal = new SimpleDie(left, right);
+
 			return new DieExpression(scal);
 		} else if (fudgeDiePattern.matcher(expString).matches()) {
-			/*
-			 * Parse fudge dice.
-			 */
+			/* Parse fudge dice. */
+			/* :SubstringIndexOf */
 			final String nDice = expString.substring(0, expString.indexOf('d'));
+			final Die fudge    = new FudgeDie(Long.parseLong(nDice));
 
-			return new DieExpression(new FudgeDie(Long.parseLong(nDice)));
+			return new DieExpression(fudge);
 		} else if (compoundDiePattern.matcher(expString).matches()) {
-			/*
-			 * Parse compound die expressions.
-			 */
+			/* Parse compound die expressions. */
 			final String[] dieParts = expString.split("c");
 
-			final DieExpression left = parseExpression(dieParts[0]);
+			/* @TODO 10/08/17 :SplitParse
+			 * 	Should this split string/parse split parts be
+			 * 	abstracted into something else that handles
+			 * 	doing the splitting correctly, as well as
+			 * 	making sure that the resulting DieExpressions
+			 * 	are of the right type?
+			 */
+			final DieExpression left  = parseExpression(dieParts[0]);
 			final DieExpression right = parseExpression(dieParts[1]);
 
+			/* :ErrorRefactor */
+			if(left.isList) {
+				System.out.printf("ERROR: Expected a scalar die expression for lhs of compound die, got a list expression instead (%s)\n", left);
+			} else if(right.isList) {
+				System.out.printf("ERROR: Expected a scalar die expression for rhs of compound die, got a list expression instead (%s)\n", right);
+			}
+
+			final Die compound = new CompoundDie(left.scalar, right.scalar);
+
 			return new DieExpression(new CompoundDie(left.scalar, right.scalar));
 		} else if (compoundingDiePattern.matcher(expString).matches()) {
-			/*
-			 * Parse compounding die expressions.
-			 */
+			/* Parse compounding die expressions. */
 			final String[] dieParts = expString.split("!!");
 
-			final DieExpression left = parseExpression(dieParts[0]);
+			final DieExpression left    = parseExpression(dieParts[0]);
 			final Predicate<Long> right = deriveCond(dieParts[1]);
 
 			final Die scal = new CompoundingDie(left.scalar, right, dieParts[1]);
+
 			return new DieExpression(scal);
 		} else if (explodingDiePattern.matcher(expString).matches()) {
-			/*
-			 * Parse exploding die expressions.
-			 */
+			/* Parse exploding die expressions. */
 			final String[] dieParts = expString.split("!");
 
-			final DieExpression left = parseExpression(dieParts[0]);
+			final DieExpression left    = parseExpression(dieParts[0]);
 			final Predicate<Long> right = deriveCond(dieParts[1]);
 
 			final DieList lst = new ExplodingDice(left.scalar, right, dieParts[1], false);
+
 			return new DieExpression(lst);
 		} else if (penetratingDiePattern.matcher(expString).matches()) {
-			/*
-			 * Parse penetrating die expressions.
-			 */
+			/* Parse penetrating die expressions. */
 			final String[] dieParts = expString.split("p!");
 
-			final DieExpression left = parseExpression(dieParts[0]);
+			final DieExpression left    = parseExpression(dieParts[0]);
 			final Predicate<Long> right = deriveCond(dieParts[1]);
 
 			final DieList lst = new ExplodingDice(left.scalar, right, dieParts[1], true);
+
 			return new DieExpression(lst);
 		} else if (diceListPattern.matcher(expString).matches()) {
-			/*
-			 * Parse simple die lists.
-			 */
+			/* Parse simple die lists. */
 			final String[] dieParts = expString.split("dl");
 
-			final DieExpression left = parseExpression(dieParts[0]);
+			final DieExpression left  = parseExpression(dieParts[0]);
 			final DieExpression right = parseExpression(dieParts[1]);
 
 			final DieList lst = new SimpleDieList(left.scalar, right.scalar);
 			return new DieExpression(lst);
 		}
 
+		/* Unhandled type of die expression. */
+		System.out.printf("INTERNAL ERROR: Valid die expression '%s' not parsed\n", expString);
 		return null;
 	}
 
-	/*
-	 * The strings and patterns used for matching.
-	 */
-
-	/*
-	 * Defines a comparison predicate.
+	/* The strings and patterns used for matching. */
+	/* @TODO 10/08/17 Ben Culkin :RegexResource
+	 * 	These regexes and patterns should be moved to something
+	 * 	external, probably using the SimpleProperties-based system that
+	 * 	BJC-Utils2 uses.
 	 */
+	/* Defines a comparison predicate. */
 	private static final String comparePoint = "[<>=]\\d+";
 
 	/*
@@ -132,9 +165,9 @@ public class DiceBox {
 	 *
 	 * This is just a number.
 	 */
-	private static final String     scalarDie               = "[\\+\\-]?\\d+sd";
-	private static final Pattern    scalarDiePattern        = Pattern.compile("\\A" +
-	                scalarDie + "\\Z");
+	private static final String     scalarDie        = "[\\+\\-]?\\d+sd";
+	private static final Pattern    scalarDiePattern = Pattern.compile(
+			String.format("\\A%s\\Z", scalarDie));
 
 	/*
 	 * Defines a simple die.