HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Dice notation roller in JavaScript

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
javascriptrollerdicenotation

Problem

I have the following function, intended to take standard dice notation (XdY+Z) and return a (sort of) random number based on the input. Are there any bugs/bad ideas/optimizable sections I am missing?

function dieRoll(dice) {
    if (/^[\d+]?d\d+[\+|\-]?\d*$/.test(dice) == false) { //Regex validate
        return "Invalid dice notation"; //Return if input invalid
    }

    if(dice[0]=="d") { //If the first character is a d (dY)
        dice = "1"+dice; //Add a 1
    }
    var minus = dice.search(/\-/); //Search for minus sign

    if (minus == -1 && dice.search(/\+/) == -1) { //If no minus sign and no plus sign (XdY)
        dice += '+0'; //Add a +0
    }
    if (minus == -1) { //If no minus sign (XdY+Z)
        var dicesplit = dice.split('+'); //Split for plus sign
        var modifier = dicesplit[1] * 1; //Number to add to total
    } else { //If there is a minus sign (XdY-Z)
        var dicesplit = dice.split('-'); //Split for minus sign
        var modifier = ("-" + dicesplit[1]) * 1; //Number to add to total
    }

    var diesplit = dicesplit[0].split('d'); //Take the first section (XdY) and split for d
    var howmany = diesplit[0] * 1; //Number of dice to roll
    var diesize = diesplit[1] * 1; //How many sides per die
    var total = 0; //Total starts as 0

    for (var i = 0; i < howmany; i++) { //Loop a number of times equal to the number of dice
        total += Math.floor(Math.random() * diesize) + 1; //Random number between 1 and diesize
    }
    total += modifier; //Add the modifier

    return total; //Return the final total
}

Solution

You validate the input string against a regular expression, but then you manually parse it again the hard way. Instead, you should use parentheses within the regular expression to define capturing groups.

I also think that your regular expression is not quite correct. Optional digits X preceding "d" should be \d* or (\d+)?. An optional ±Z should be ([+-]\d+)?.

function dieRoll(dieSpec) {
    var match = /^(\d+)?d(\d+)([+-]\d+)?$/.exec(dieSpec);
    if (!match) {
        throw "Invalid dice notation: " + dieSpec;
    }

    var howMany = (typeof match[1] == 'undefined') ? 1 : parseInt(match[1]);
    var dieSize = parseInt(match[2]);
    var modifier = (typeof match[3] == 'undefined') ? 0 : parseInt(match[3]);

    …
}

Code Snippets

function dieRoll(dieSpec) {
    var match = /^(\d+)?d(\d+)([+-]\d+)?$/.exec(dieSpec);
    if (!match) {
        throw "Invalid dice notation: " + dieSpec;
    }

    var howMany = (typeof match[1] == 'undefined') ? 1 : parseInt(match[1]);
    var dieSize = parseInt(match[2]);
    var modifier = (typeof match[3] == 'undefined') ? 0 : parseInt(match[3]);

    …
}

Context

StackExchange Code Review Q#40993, answer score: 5

Revisions (0)

No revisions yet.