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

Simple randomization program

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

Problem

I'm a few months into learning C++ programming and I want to know if I'm moving generally in the right direction or not with the following code. This is the most advanced thing I've created so far, but it doesn't contain any pointers or references so I'm worried I'm not doing things properly on the memory level. I especially want advice regarding moving this type of code to templates and virtual functions.

```
//This program's purpose is a simple battle simulator and a simple lotto simulator
//This requires two classes and a bit of procedural main game logic that can repeat

//The first class is a Randomizer class that allows the user to generate
//random numbers by inputting the desired amount of numbers, and the max
//range for those numbers. The default start value is 1.

//The second class is a Battler class that contains two Randomizer objects.
//It also contains a method to compare the second integer of each of the vector
//arrays contained in each of those objects, and returns -1, 0, or 1 depending
//on which value is greater, or 0 if they are tied.

#include //Necessary for cout and endl
#include //Necessary for time_t variable
#include //Necessary for rand and srand
#include //Necessary for vector

/HEADER OF RANDOMIZER CLASS/
class Randomizer //The preferred syntax is to start with a capital letter for classes
{
private:
time_t timevalue; //time_t member variable that will be set later
void randomTimeInit(); //called by constructor (runs once)

public:
Randomizer() //Class constructor
{
srand(time(NULL)); //This is JUST necessary here to get new results every time
//Caution: Only run once per program execution! (per object?)
void randomTimeInit(); //This function sets time_t value and seeds rand() with it
std::cout randomIntArray; //The vector array for the random numbers
int getRandomNumber(int); //Function to r

Solution

I suggest the following:

-
Member variables should be private.Do you really need

std::vector randomIntArray


to be public in Randomizer?

-
Be const correct. Declare variables to be const whenever possible. The ff.

int getRandomNumber(int); 
void createRandomIntArray(int, int);


can be rewritten to:

int getRandomNumber(const int);
void createRandomIntArray(const int, const int);


-
In terms of OOP design, I think it would be better if you could create another class named BattleGame ( or any other name ) where you will put your game logic.

class BattleGame 
{
public:
    void chooseBattleOrLotto();

private:
    void inputAndPrintRandomIntArray();
    void lottoLoop(); 
    void battleLoop();
 }


You can then call it using:

int main(int argc, char* args[]) 
{
    BattleGame b;
    b.chooseBattleOrLotto();
    return 0;
}


This encapsulates the methods that the caller does not need to know.

There are probably more things you can do to improve your program. I will post some more suggestions when I get more time.

Code Snippets

std::vector<int> randomIntArray
int getRandomNumber(int); 
void createRandomIntArray(int, int);
int getRandomNumber(const int);
void createRandomIntArray(const int, const int);
class BattleGame 
{
public:
    void chooseBattleOrLotto();

private:
    void inputAndPrintRandomIntArray();
    void lottoLoop(); 
    void battleLoop();
 }
int main(int argc, char* args[]) 
{
    BattleGame b;
    b.chooseBattleOrLotto();
    return 0;
}

Context

StackExchange Code Review Q#13261, answer score: 6

Revisions (0)

No revisions yet.