# Dice notation roller in JavaScript

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=="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; //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; //Number to add to total } var diesplit = dicesplit.split('d'); //Take the first section (XdY) and split for d var howmany = diesplit * 1; //Number of dice to roll var diesize = diesplit * 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 }  ## 1 Answer 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 == 'undefined') ? 1 : parseInt(match);
var dieSize = parseInt(match);
var modifier = (typeof match == 'undefined') ? 0 : parseInt(match);

…
}

• The two corrections you suggested are valid points; thanks! The group-capturing RegEx seems to work as well. I'm a little confused by the next bit - what exactly are the ? 1 : and ? 0 : parts doing? Feb 5, 2014 at 20:09
• If omitted, you want the X to default to 1, and Z to default to 0. Feb 5, 2014 at 20:17