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

Simple Dice Roll game

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

Problem

Is there a more efficient algorithm to generate random numbers in C++?

(This code is working on Dev-C++, but I'm not sure if it works on a Borland compiler.)

```
/*
Author: Arpit Agrawal
Email: arpitagrawal294@gmail.com
Description: Dice Roll Algorithm.
Project Name: e-Roll.

*/

#include
#include
#include
#include
#include
#include

void call();
void one();
void two();
void three();
void four();
void five();
void six();
void call();
int main()
{

//gotoxy(30,15);
cout<<"\n\n\n\n\t\tAuthor: Arpit Agrawal\n\t\tEmail: arpitagrawal294@gmail.com\n\t\tDescription: Dice Roll Algorithm.\n\t\tProject Name: e-Roll.\n\t\t" ;
cout<<"\n\n\t\tLoading. . . . . . . ";
Sleep(3000);
cout<<"\n\n\t\tPress r to roll or q to quit the game "<<endl;
char ch;
ch = getch();
xm:
if (ch=='r'){
system("cls");
call(); }
else
exit (0);
cout<<endl<<endl<<"Press r to roll again q to quit!";
ch = getch();
goto xm;
getch();
}

void call()
{
srand (time(NULL));

int n;
n= rand();
n = 1 + n % 6;

switch (n)
{
case 1:
one();
break;
case 2:
two();
break;
case 3:
three();
break;
case 4:
four();
break;
case 5:
five();
break;
case 6:
six();
break;
default:
cout<<"NONUM";

}
}

void one()
{
cout << " -----" << endl;
cout << "| |" << endl;
cout << "| O |" << endl;
cout << "| |" << endl;
cout << " -----" << endl;
}
void two()
{
cout << " -----" << endl;
cout << "| O|" << endl;
cout << "| |" << endl;
cout << "|O |" << endl;
cout << " -----" << endl;
}
v

Solution

Only call srand() once in an application:

srand (time(NULL));


Should be just after main() starts.

This does not generate an evenly distributed the random numbers.

n= rand();
    n = 1 + n % 6;


This is because rand() returns a number from [0,RAND_MAX) or [0,32767) which is not exactly divisible by 6. So you get:

1:  1/5462        Notice this is one more than the others.
    2:  1/5461
    3:  1/5461
    4:  1/5461
    5:  1/5461
    6:  1/5461


Probably not an issue for a simple app but worth noting. The proper way to do this is:

int dieRoll() // 1-6 evenly distributed.
{
     static int const max = RAND_MAX/6*6;

     int r = rand();
     while(r >= max) { r = rand();}

     return r%6+1;
}


Probably best not to use goto:

xm:
if (ch=='r'){
system("cls"); 
call();  }
else
exit (0);
cout<<endl<<endl<<"Press r to roll again q to quit!";
ch = getch();
goto xm;


Prefer (a standard loop):

while (ch=='r') {
    system("cls"); 
    call();

    cout<<endl<<endl<<"Press r to roll again q to quit!";
    ch = getch();
}


Lets also compress your switch statement:

switch (n) {
        case 1:   one();break;
        case 2:   two();break;
        case 3: three();break;
        case 4:  four();break;
        case 5:  five();break;
        case 6:   six();break;
        // We know the number will never be anything else
        // so don't need the default.
    }


Don't need to use so many std::endl.

cout << " ----- \n"
         << "|O   O|\n"
         << "|     |\n"
         << "|O   O|\n"
         <<  " -----" << endl;


std::endl is used to flush the output. If you just want a new line use "\n".

Code Snippets

srand (time(NULL));
n= rand();
    n = 1 + n % 6;
1:  1/5462        Notice this is one more than the others.
    2:  1/5461
    3:  1/5461
    4:  1/5461
    5:  1/5461
    6:  1/5461
int dieRoll() // 1-6 evenly distributed.
{
     static int const max = RAND_MAX/6*6;

     int r = rand();
     while(r >= max) { r = rand();}

     return r%6+1;
}
xm:
if (ch=='r'){
system("cls"); 
call();  }
else
exit (0);
cout<<endl<<endl<<"Press r to roll again q to quit!";
ch = getch();
goto xm;

Context

StackExchange Code Review Q#31980, answer score: 14

Revisions (0)

No revisions yet.