How 'far off' is this code?
I wrote a class for simply creating sets of dice, and rolling them. This could obviously be pretty useful for anything involving dice. I also have the concept of an open-ended roll, which is simply a roll with no "hard" cap. For example, on a d10, if you rolled a 10, you would roll the die again, and subtract 1 from that roll and add it. If you get another 10, you'd add yet another roll(being0-9 for the secondary rolls.) I first saw a reference to this on the forums for a game called Dominions 2, but I am unsure where it originates. I am not claiming it was my idea at any rate :D
I haven't added the ability to remove dice from the dice pool yet, I'll consider it if I find I need that ability (I suspect I will simply build and allow the scope to eat them.) It has the ability to add new dice at any time, and it has a Variadic *yay for new words :p * constructor. I am also fairly certain it doesn't leak, but I am too new to claim 100% certainty.
I like defining all my functions inside the class, but I am aware that can cause the compiler to inline them, which isn't always good. What I am driving at here is, how much would this code stick out as poor in a "real"(as in I was getting paid) program. I am not naive enough to think it is up to pro snuff, I just was curious how far off it is, and what is *not* up to snuff about it.
Here is the class itself: dice.cpp
Code:
#include<cstdlib>
#include<stdarg.h>
class Dice
{
private:
//Hidden DIE class for use ONLY with DICE pools
class Die
{
public:
unsigned char size,face;bool open;Die * next;
//Rolls the DIE
int Roll(void)
{
if(open)//If open go through the motions for an OE dice roll
{
int tempRoll=0,tempFace;tempFace=(rand()%size)+1;tempRoll+=tempFace;
if(tempFace==size){do{tempFace=(rand()%size);tempRoll+=tempFace;}while(tempFace==size-1);};
face=tempRoll;
}
else//Otherwise, just roll and report
{
face=(rand()%size)+1;
}
}
};
Die * root;//Root pointer for lists of DIE
public:
Dice(){;}
Dice(char noDice...)//Handy-dandy Variadic constructor
{
va_list ap;
va_start(ap,noDice);
char size;bool open;
while(noDice-->0)
{
size=(char)va_arg(ap,int);open=(bool)va_arg(ap,int);
this->Add(size,open);
}
va_end(ap);
}
~Dice()//Destructor. I hope it's airtight:)
{
Die * finger=root;Die * nullNext;
while(!(finger==NULL)){nullNext=finger->next;delete finger;finger=nullNext;}
}
void Roll(int times)//Rolls all DIE a certain number of times. Will eventually
{ //add ability to roll X times and take highest/lowest
Die * finger=root;
for(int x=1;x<=times;x++){while(!(finger==NULL)){finger->Roll();finger=finger->next;};};
}
int Roll(void)//Overloaded to show what roll the DICE pool currently contains
{
Die * finger=root;
int total=0;
while(!(finger==NULL)){total+=finger->face;finger=finger->next;}
return total;
}
void Add(char Size,bool Open=false)//Allows the addition of DIE after the constructor
{
Die * newDie;newDie = new Die;newDie->size=Size;newDie->open=Open;
if(root==NULL){root=newDie;newDie->next=NULL;}
else
{
Die * finger;finger=root;
while(!(finger->next==NULL)){finger=finger->next;}
finger->next = newDie;newDie->next=NULL;
}
}
};
Here is a sloppy (not interested in comments on THIS part, but just posted it to show a use, I know it's *super* ghetto) MAIN to go with it: main.cpp
Code:
//500,000 roll dice statistic test
#include<iostream.h>
#include"dice.cpp"
int main(void)
{
bool addMore;
Dice * Derka = new Dice();
do
{
srand((unsigned)time(0));
int size; char t_f;
printf("Enter a die size(1-255):");
scanf("%d",&size);
printf("Is this die open ended(T/F)?:");
cin>>t_f;
if (t_f=='f' || t_f=='F'){Derka->Add((unsigned char)size);}
else {Derka->Add((unsigned char)size,true);}
printf("Add more dice?(T/F):");
cin>>t_f;
if (t_f=='f' || t_f=='F'){addMore=false;}else{addMore=true;};
}while(addMore==true);
//******Dice Statistics******
int stats[500000];
int largest=0,smallest=999999;
for(int x=1;x<=500000;x++)
{
Derka->Roll(1);
if(Derka->Roll()>largest){largest=Derka->Roll();}
if(Derka->Roll()<smallest){smallest=Derka->Roll();}
stats[x-1]=Derka->Roll();
//printf("%3d|",Derka->Roll());
//if(x%10==0){printf("\n");printf("----------------------------------------\n");}
//if (x%100000==0){printf(".");};
}
printf("\n%3d - %3d Range\n",smallest,largest);
for(int x=smallest;x<=largest;x++)
{
int rollTemp=0;
for (int y=1;y<=500000;y++)
{
if(stats[y-1]==x){++rollTemp;}
}
printf("%3d rolled %6d times and %2d%% of the time.\n",x,rollTemp,(rollTemp/5000));
}
system("PAUSE");
delete Derka;
}
Remember, I KNOW the main.CPP is sloppy, just want comments on dice.cpp ;)
Ok I *thought* I had this down but..
Ok, I *thought* I had this, but something went horribly wrong. :(
I am trying to straighten this mess out, but now I get errors.
Multiple definition of `Dice::_______ for ALL methods.
Now, I don't claim to be some kind of old hand at this, and I realize this might be some kind of RTFM error, but seriously I can't get what I am doing wrong from any of my sources.
Code:
#include<cstdlib>
#include<stdarg.h>
//**********************************************
//*Dice class used for pooling and rolling DIE.*
//**********************************************
class Dice
{
public:
Dice(){};
Dice(char...);//Variadic constructor
~Dice(void);
int Roll(void);
void Roll(int);
void Add(char,bool);
private:
//*********************************************
class Die//Hidden DIE class for use ONLY with DICE pools
{
public:
void roll(void);//An internal function for reading DIE.
unsigned char size;//Size is the d<x>(d6,d10,d20 etc)
unsigned char face;//Face is the current rolled number of the die.
bool open; //Open determines if the roll is OPEN
//An open roll has no firm limit, but is
//unlikely to roll *TOO* much higher than
//a standard roll.
Die * next;//A pointer to the next DIE for linking lists.
};
//*********************************************
Die * root;//Root pointer for lists of DIE
};
//**********************************************
//*Methods**************************************
//**********************************************
Dice::Dice(char noDice...)//Variadic constructor
{
va_list ap;
va_start(ap,noDice);
char size;bool open;
while(noDice-->0)
{
size=(char)va_arg(ap,int);open=(bool)va_arg(ap,int);
this->Add(size,open);
}
va_end(ap);
}
Dice::~Dice(void)//Destructor.
{
Die * finger=root;Die * nullNext;
while(!(finger==NULL)){nullNext=finger->next;delete finger;finger=nullNext;}
}
int Dice::Roll(void)//Overloaded to show what roll the DICE pool currently contains
{
Die * finger=root;
int total=0;
while(!(finger==NULL)){total+=finger->face;finger=finger->next;}
return total;
}
void Dice::Roll(int times)//Rolls all DIE a certain number of times. Will eventually
{ //add ability to roll X times and take highest/lowest.
Die * finger=root;
for(int x=1;x<=times;x++){while(!(finger==NULL)){finger->roll();finger=finger->next;};};
}
void Dice::Add(char Size,bool Open=false)//Allows the addition of DIE after the constructor
{
Die * newDie;newDie = new Die;newDie->size=Size;newDie->open=Open;
if(root==NULL){root=newDie;newDie->next=NULL;}
else
{
Die * finger;finger=root;
while(!(finger->next==NULL)){finger=finger->next;}
finger->next = newDie;newDie->next=NULL;
}
}
void Dice::Die::roll(void)
{
if(open)//If open go through the motions for an OE dice roll
{
int tempRoll=0,tempFace;tempFace=(rand()%size)+1;tempRoll+=tempFace;
if(tempFace==size){do{tempFace=(rand()%size);tempRoll+=tempFace;}while(tempFace==size-1);};
face=tempRoll;
}
else//Otherwise, just roll and report
{
face=(rand()%size)+1;
}
}
I feel kind of dirty asking for help, knowing this is probably borderline RTFM, but what went wrong when I decided to move these out of inline? Yes, I know INLINE is a compiler hint and doesn't necessarily mean the same thing as "in the {} for the class," but I know that makes them INLINE by default.
EDIT: I figured it out. Apparently #including and adding the same file to the project is a bad idea :eek:
