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 

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='0' && diceString[i]='0' && diceString[i]='0' && diceString[i]<='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 > 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 > 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 > 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.

StackExchange Code Review Q#41403, answer score: 11