cleanup

src/main/kotlin/simulation/dice/DiceRoller.kt

@@ -8,8 +8,8 @@ import kotlin.math.min
 /**
  * Enumeration of different dice roll types.
  *
- * @property Advantage Rolls the dice twice and takes the higher result.
  * @property Normal Rolls the dice normally once.
+ * @property Advantage Rolls the dice twice and takes the higher result.
  * @property Disadvantage Rolls the dice twice and takes the lower result.
  */
 enum class RollType {
@@ -33,6 +33,10 @@ data class RollResult(val min: Int, val max: Int, val result: Int)
 
 
 interface DiceRoller {
+    /**
+     * string representation of the base dice in {n}d{size} format
+     * i.e. 1d20, 2d7, 30d100
+     */
     val rollString: String
     val modifiers: List<DiceModifier<Int>>
     val nDice: Int
@@ -40,7 +44,7 @@ interface DiceRoller {
 
 
     companion object {
-        internal fun defaultDiceStringParseFn(rollString: String): Pair<Int, Int> {
+        fun defaultDiceStringParseFn(rollString: String): Pair<Int, Int> {
             val cleanRollString = rollString.lowercase()
             val parts = cleanRollString.split('d')
             return Pair(parts[0].toInt(), parts[1].toInt())

src/test/kotlin/simulation/DiceTest.kt

@@ -82,26 +82,32 @@ internal class DiceRollerTests {
         verifyBoundariesForTypes(1, 20)
         verifyBoundariesForTypes(2, 6)
         verifyBoundariesForTypes(5, 8)
-        verifyBoundariesForTypes(1000, 6)
+
-        verifyBoundariesForTypes(6, 1000)
+        //we have to increase the iteration count by a few orders of magnitude in order to have a chance of hitting a max
+        // roll w/disadvantage
+        verifyBoundariesForTypes(6, 1000, 100_000_000)
+        verifyBoundariesForTypes(1000, 6, 100_000_000)
+
     }
 
 
-    private fun verifyBoundariesForTypes(nDice: Int, dieSize: Int) {
+    private fun verifyBoundariesForTypes(nDice: Int, dieSize: Int, iterationCount: Long = 10_000) {
         val rollString = "${nDice}d${dieSize}"
-        val iterations = 10_000_000
         val max = nDice * dieSize
 
-        var observedMin = false
-        var observedMax = false
-
-
         RollType.entries.parallelStream()
             .forEach {
+                var observedMin = false
+                var observedMax = false
                 val dice = Dice.regular(rollString)
                 val r = Random(1)
-                for (i in 0..<iterations) {
+
+                for (i in 0..<iterationCount) {
                     val res = dice.roll(r, it)
+
+                    //make sure it isn't bigger or smaller than it's supposed to be
+                    Assertions.assertTrue(res.result in nDice..max)
+
                     if (!observedMin && res.result == nDice) {
                         observedMin = true
                     }
@@ -112,17 +118,18 @@ internal class DiceRollerTests {
                         break
                     }
                 }
+
                 Assertions.assertTrue(observedMin)
                 Assertions.assertTrue(observedMax)
             }
     }
 
     @Test
-    fun verifyNormalRollWithinExpectedBounds() {
+    fun verifyNormalRollMeanWithinExpectedBounds() {
         val n = 2
         val max = 100
         val rollString = "${n}d${max}"
-        val tolerance = 0.05 //we expect more than a 25% improvement
+        val tolerance = 0.05 //we want a "wiggle" around the mean of < 5%
         val iterations = 100_000_000
 
         val expectedAverageLowerBound = ((n + (n * max)) / 2) * (1 - tolerance)