Simple dice roll using std::string as dice notation This will be a function that returns a random number from Dice Notation (more on Wikipedia).  I will later use this function to determine whether some skill check is passed or not.


Is there any need for optimization?
If so, how?
Any other thoughts about this code?




#include &lt;string&gt;

using namespace std;

int toInt(string text) {
    return atoi(text.c_str());
}

int main(int argc, char* argv[]) {

    srand(time(NULL));

    string diceString = "1d6+4";
    unsigned int dice1number = 0;

    string info = "dice ["+diceString+"] = %i\n";
    printf("==[START]================\n");

    printf(info.c_str(),dice1number);

    // parse dice
    unsigned int i=0;
    unsigned int part = 1;

    string dicePart1 = "";
    string dicePart2 = "";
    string dicePart3 = "";

    for(i=0;i&lt;diceString.length();i++) {

        if (diceString[i]=='d') {
            part = 2;
            continue;
        }

        if (diceString[i]=='+') {
            part = 3;
            continue;
        }

        if (diceString[i]=='-') {
            dicePart3 += "-";
            part = 3;
            continue;
        }

        if (part==1 &amp;&amp; diceString[i]&gt;='0' &amp;&amp; diceString[i]&lt;='9') {
            dicePart1 += diceString[i];
        }

        if (part==2 &amp;&amp; diceString[i]&gt;='0' &amp;&amp; diceString[i]&lt;='9') {
            dicePart2 += diceString[i];
        }

        if (part==3 &amp;&amp; diceString[i]&gt;='0' &amp;&amp; diceString[i]&lt;='9') {
            dicePart3 += diceString[i];
        }

    }

    printf("dicePart1 = [%s]\n",dicePart1.c_str());
    printf("dicePart2 = [%s]\n",dicePart2.c_str());
    printf("dicePart3 = [%s]\n",dicePart3.c_str());

    int roll = 0;
    int add = toInt(dicePart3);
    roll = toInt(dicePart1) * (rand() % toInt(dicePart2)) + add;
    printf("roll = [%i]\n",roll);

    printf("==[END]==================\n");

    return 0;
}
Bug

Your code has an integer-division/multiplication problem/bug.

Consider the dice-roll specification 10d10+0.

Your code will parse this down to:

string dicePart1 = "10";
string dicePart2 = "10";
string dicePart3 = "0";


and this will be used as:

int add = toInt(dicePart3);
roll = toInt(dicePart1) * (rand() % toInt(dicePart2)) + add


At face value, this may be OK, but, in reality, the calculation boils down to:

roll = 10 * (rand() % 10) + 0;


This will only ever be able to produce 10 results, 0, 10, 20, 30, ...., 90, which really is not what you want.

You really should do this as a loop, or as a floating point operation.... the loop seems to be easier:

roll = toInt(dicePart3);
for (i = toInt(dicePart1); i &gt; 0; i--) {
    roll += (rand() % toInt(dicePart2));
}


Second Bug

In addition to the bug I pointed out above, I have actually identified a second bug. Dice are always 1-based.... you cannot roll a '0'. So, the method would actually have to be:

roll = toInt(dicePart3);
for (i = toInt(dicePart1); i &gt; 0; i--) {
    roll += 1 + (rand() % toInt(dicePart2));
}


Discussion on distribution...

You may be tempted to be more efficient and use the function:

int p1 = toInt(dicePart1);
int p2 = toInt(dicePart2);
int p3 = toInt(dicePart3);

roll = p1 + (rand() % (p1 * p2)) + p3;


The above will create a result that is distributed in the correct range.... but the actual distribution curve is flat.... the odds of a 10d10+0 being 10 is the same as it being 50.... but, in reality, there are many ways to throw a 50, but only 1 way to throw a 10. As a result, the roll-value of 10d10+0 is not evenly distributed in the range 10 ... 100. You need to use a method where each dice is rolled individually.

int p1 = toInt(dicePart1);
int p2 = toInt(dicePart2);
int p3 = toInt(dicePart3);

....

roll = p3;
for (i = p1; i &gt; 0; i--) {
    roll += 1 + (rand() % p2);
}


The above will, for 10d10+0 produce values in the correct range 10 ... 100 and the value frequencies will be distributed normally, as expected.