Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I created a simple number-guessing game. Do I need to divide this program into functions? Please offer suggestions.

#include<iostream>
#include<string>
#include<ctime>

void drawLine(int n, char symbol);
void rules();

int main()
{
    int amount;
    int bettingAmount;
    int guess;
    int dice;
    std::string playerName;

    drawLine(60,'_');
    std::cout << "\n\n\n\t\tCASINO GAME\n\n\n\n";
    drawLine(60,'_');
    std::cout << "\n\nEnter Your Name : ";
    std::getline(std::cin, playerName);
    std::cout << "\n\nEnter Deposit amount to play game : ";
    std::cin >> amount;
    char choice;
    do
    {
        system("cls");
        rules();
        std::cout << "\n\nYour current balance is $ " << amount << "\n";

        do
        {
            std::cout <<playerName<<", enter money to bet ";
            std::cin >> bettingAmount;
            if(bettingAmount > amount)
                std::cout << "Your betting amount is more than your current balance\n\nRe-enter data\n ";
        }while(bettingAmount > amount);

        do
        {
            std::cout << "Guess your number to bet between 1 to 10 :";
            std::cin >> guess;
            if(guess <= 0 || guess > 10)
                std::cout << "Please check the number!! should be between 1 to 10\n\nRe-enter data\n ";
        }while(guess <= 0 || guess > 10);

        srand(time(0));
        dice = rand()%10 + 1;

        if(dice == guess)
        {
            std::cout << "\n\nGood Luck!! You won Rs." << bettingAmount * 10;
            amount = amount + bettingAmount * 10;
        }
        else
        {
            std::cout << "Bad Luck this time !! You lost $ "<< bettingAmount <<"\n";
            amount = amount - bettingAmount;
        }

        std::cout << "\nThe winning number was : " << dice <<"\n";
        std::cout << "\n"<<playerName<<", You have $ " << amount << "\n";
        if(amount == 0)
        {
            std :: cout << "You have no money to play ";
            break;
        }
        std::cout << "\n\n-->Do you want to play again (y/n)? ";        
        std::cin >> choice;
    }while(choice =='Y'|| choice=='y');

    std::cout << "\n\n\n";
    drawLine(70,'=');
    std::cout << "\n\nThanks for playing game. Your balance amount is $ " << amount << "\n\n";
    drawLine(70,'=');

    return 0;
}

void drawLine(int n, char symbol)
{
    for(int i=0; i<n; i++)
        std::cout << symbol;
    std::cout << "\n" ;
}

void rules()
{
    system("cls");
    std::cout << "\n\n";
    drawLine(80,'-');
    std::cout << "\t\tRULES OF THE GAME\n";
    drawLine(80,'-');
    std::cout << "\t1. Choose any number between 1 to 10\n\t2. If you win you will get 10 times of money you bet\n\t3. If you bet on wrong number you will lose your betting amount\n\n";
    drawLine(80,'-');
}

// END OF PROGRAM
share|improve this question

2 Answers 2

Very good.

A few things I would change:

  1. Declare variables as close to the point of usage as possible.

    int amount;
    int bettingAmount;
    int guess;
    int dice;
    std::string playerName;
    

    This is very C like to declare all your variables at the top.
    By declaring them close to the point of usage you don't need to look around for them they should be close to where you are using them. Also C++ has the concept of construction/destruction on objects which comes in useful and is applies at the point of creation.

  2. Validate that your read worked.

    std::cin >> amount;
    

    If the user types in "Kevin" then your whole program will stop working. Any time you have user input it is best to read a line of text (as users interact with the terminal a line at a time). Then parse and validate that data out of line (so that failure does not set the bad bit on the stream (because you will need to reset that).

    while(true)
    {
        std::string   userInput;
        std::getline(std::cin, userInput);
    
        std::stringstream userInputStream(userInput);
        if (userInputStream >> amount)
        {
            // The user entered a valid number and not text.
            // 
            // If you care you may want to check that they did not
            // enter text after the number like "10 Bob"
            // Personally I would not bother for version one
    
            break;   // exit the loop.
        }
        else
        {
            // invalid Input
            std::cerr << "Invalid Input: Try again\n";
            // Loop re-enters.
            // User trys again.
        }
    }
    
  3. I prefer all sub blocks to have {} around them

    if(bettingAmount > amount)
        std::cout << "Your betting amount is more than your current balance\n\nRe-enter data\n ";  
    
    // I would write like this:
    if(bettingAmount > amount)
    {
        std::cout << "Your betting amount is more than your current balance\n\nRe-enter data\n ";
    } 
    

    This makes it easier to read. Also when modifying the code and needing to add more statements you don't need to remember to add the {}.

  4. Don't re-seed rand.

    srand(time(0));
    

    You only need to do this once in the application. Do it as main() is entered. Don't do it again. It breaks the randomness. It's not required for a simple application like this. But if you want to do better then I would look at using the new random number generates that are part of C++11. It will be good practice (rand() is not very good).

Even so the overall structure is still very C-Like.
You could practice your OO style here.

int main()
{
    Game       game;

    HumPlayer  human;
    while(!human.hadEnough())
    {
        game.play(human);
    }

    CompPlayer computer;
    while(!computer.hadEnough())
    {
        game.play(computer);
    }

    std::cout << (human.score() < computer.score())
               ? "The time of the computer overlords has arrived.\n";
               : "Still time before this silly machines are smart.\n";
}
share|improve this answer
    
I would add that that program will run as expected only on Windows. I'm talking about system("cls"); –  enedil Jun 10 at 21:44
    
@enedil: It will still run fine on linux. There is no check on the result of the system command so you may get spurious output and the screen may not clear but the main functionality of the program will work just fine. –  Loki Astari Jun 10 at 21:46
    
Yes, I know, but in my opinion, when an application works, it behaves all the time as expected. –  enedil Jun 10 at 21:50
    
@enedil: touché –  Loki Astari Jun 10 at 22:17

Yes, you should divide this into functions. Right now you just have two functions that print lines and such to the screen, which aren't too useful in my opinion. Other than that, main() is doing most of the work, so having additional functions is a good idea.

You can have separate functions for the following tasks:

  • getting the initial input (name and deposit amount)
  • displaying the current balance and other info
  • getting the next number from the user

You could have all of this within a loop in main(). After the game ends, you can display the results in the same function, or have a separate function for that as well.

Additional notes:

  • You should move srand() to the top of main(). Doing so will help ensure that it's not called multiple times, otherwise you'll receive the same random number each time.
  • Try not to bloat the code with additional line and newline printing. You may consider utilizing the <iomanip> library, which may include some things to your liking.
  • If you want to improve the structure of the game even more, consider making a Game class instead. It would look quite different from this, but it's the next step up for making a clean implementation.
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.