2 quick questions

Currently working through the Jumping Into C++ E-book, on the chapter about randomizing, one of the practice problems has asked me to make a "slot machine" style output and then have rewards for different results. The code below is what I have come up with, but I know it can be better. First question : the system("cls") I have read around and realize that system calls are often frowned upon, BUT I honestly can't think of a way to get this to "look like a slot machine" without clearing the window out each time, any ideas on how to do this without the system call? It does really slow it down and would be nice to be rid of... Second question : any ideas on how I could write the code better(shorter) for the rewards? I wish switches could take in multiple variable dependencies but alas they cannot :/.

Code:

#include <iostream>
#include <stdlib.h>
#include <ctime>


using namespace std;


int randNum0 (int low, int high)
{
    return rand() % (high-low) + low;
}
int randNum1 (int low, int high)
{
    return rand() % (high-low) + low;
}
int randNum2 (int low, int high)
{
    return rand() % (high-low) + low;
}


int main()
{
    int num0;
    int num1;
    int num2;
    for (int i = 0; i < 100; ++i)
    {
        srand (time(NULL));
        int num0 = randNum0(1, 9);
        int num1 = randNum1(1, 9);
        int num2 = randNum2(1, 9);
        system("CLS");
        cout << "-------" <<endl;
        cout << "|" << num0 << "|" << num1 << "|" << num2 << "|\n";
        cout << "-------" <<endl;
    }

    if (num0 == 1 && num1 == 1 && num2 == 1)
    {
        cout << "You won " <<num0 <<"dollars. Congratulations!\n";
    }
    if (num0 == 2 && num1 == 2 && num2 == 2)
    {
        cout << "You won " <<num0 <<"dollars. Congratulations!\n";
    }
    if (num0 == 3 && num1 == 3 && num2 == 3)
    {
        cout << "You won " <<num0 <<"dollars. Congratulations!\n";
    }
    if (num0 == 4 && num1 == 4 && num2 == 4)
    {
        cout << "You won " <<num0 <<"dollars. Congratulations!\n";
    }
    if (num0 == 5 && num1 == 5 && num2 == 5)
    {
        cout << "You won " <<num0 <<"dollars. Congratulations!\n";
    }
    if (num0 == 6 && num1 == 6 && num2 == 6)
    {
        cout << "You won " <<num0 <<"dollars. Congratulations!\n";
    }
    if (num0 == 7 && num1 == 7 && num2 == 7)
    {
        cout << "You won " <<num0 <<"dollars. Congratulations!\n";
    }
    if (num0 == 8 && num1 == 8 && num2 == 8)
    {
        cout << "You won " <<num0 <<"dollars. Congratulations!\n";
    }
    if (num0 == 9 && num1 == 9 && num2 == 9)
    {
        cout << "You won " <<num0 <<"dollars. Congratulations!\n";
    }


}

For terminal/console stuff seek out some form of the "curses" library.

The functions `randNum0', `randNum1', and `randNum2' are all the same mechanisms. You don't have to have but one of them.

The slot machine as you've designed it essentially only checks if the three numbers are the same. Can you think of a better way to do that?

Soma

When you say "int num0;" inside main, you're declaring a variable called num0 but not setting it to anything. When you say

Code:

int num0 = randNum0(1, 9);

inside the for loop, you are declaring another variable called num0 which hides the outer variable, so the outer variable is still not set to anything. Thus when the for loop ends, the num* variables will be uninitialized! You're not actually ever assigning anything to these variables, and you never read user input into them.

As for the code that looks like this,

Code:

if (num0 == 1 && num1 == 1 && num2 == 1)

you can compare anything of the same type with the == operator. So you can write "1 == 1", or "num0 == 1", or "1 == num0", or "num0 == num1"... by putting a variable on each side of the == operator you can probably come up with some much shorter code.

I'm not certain why you made three functions that do the same thing, but my guess is that you didn't want them interfering with each other. But you should know that in C++, each function call is independent of the others. When you pass in parameters to a function, the parameters can take on any name, and they'll never interfere with variables in other functions. Parameters are passed by value; their names are irrelevant. Similarly for calling a function. You can call the same function in different places and the function will be okay with that (unless it relies on global variables or something). It's just like calling rand(). Clearly each time you call rand() you're getting a different number, and they don't interfere with each other.

Cheers.

In addition to what's already been said, you should put the srand call BEFORE the loop. It is generally placed so it executes only ONCE in the entire execution of the program, setting the original seed to a "random" number.

Also, your randNum function is incorrect. With the inputs 1 and 9 it will return a number between 1 and 8, inclusive, which is not what you seem to want. It should be rand() % (high-low+1) + low.

And I just noticed that you're redefining ints num0, num1, and num2 within the loop, which will "hide" the outer ones and the outer ones will not get the values. It's the outer ones your testing in your pile of if statements.

Originally Posted by rTeapot

First question : the system("cls") I have read around and realize that system calls are often frowned upon, BUT I honestly can't think of a way to get this to "look like a slot machine" without clearing the window out each time, any ideas on how to do this without the system call? It does really slow it down and would be nice to be rid of...

Hmmm. I am not sure! Here's the FAQ: FAQ > Clear the screen? - Cprogramming.com

system() is indeed frowned upon for several good reasons (mentioned in the FAQ), so it's definitely best to avoid it if you can. I think you'll find that the slowdown introduced by the slow system() call is what makes your program "look like a slot machine" Without that performance penalty, the program will run so fast you won't get to see anything! So you'll need to introduce a delay.

It depends how serious you are about doing the right thing. I'd be tempted to just use system().... so long as you KNOW it's bad, it's not so bad maybe. *shudder* maybe someone else will have a better idea.

Originally Posted by rTeapot

Second question : any ideas on how I could write the code better(shorter) for the rewards? I wish switches could take in multiple variable dependencies but alas they cannot :/.

You're currently testing all the numbers are the same, so you could just do:

Code:

 if (num0 == num1 && num1 == num2)   
    {
        cout << "You won " <<num0 <<"dollars. Congratulations!\n";
    }

Nothing wrong with having a load of if statements, though since only one case will be true it'd be more normal to use:

Code:

if(cond) 
..
else if (cond) 
..
else if (cond)

If you wanted to define lots of different winning combos and have them in an easy to read layout, I'd probably go for holding the winning combinations in a 2D array or array of structs along with any supporting information (e.g. amount won), then writing the code to loop through the array.

Couple of code comments:

Code:

int main()
{
    int num0;
    int num1;
    int num2;
    for (int i = 0; i < 100; ++i)
    {
        srand (time(NULL));

You only need to call srand once - it shouldn't be in the loop.

Code:

        int num0 = randNum0(1, 9);
        int num1 = randNum1(1, 9);
        int num2 = randNum2(1, 9);

I don't think this should work -- declaring num0,num1,num2 in the inner loop then using them in the outer loop. But apparently it does - anybody?

[edit] - ignore that last comment -- user error. It behaves as expected (i.e. the values are not valid outside of the for loop).

Originally Posted by smokeyangel

You only need to call srand once - it shouldn't be in the loop.

That's understating it a little.
Calling it more than once, such as in the manner seen here, is harmful to your ability to actually get random numbers. You could easily get the same three values repeated up to 100 times by doing it the way its done here.

And can we drop this nonsense about "quick questions" (if I could, I'd be aiming this at ALL posters).
What people really mean (perhaps subconciously) when they say that, is that they want a quick answer!
Tough luck to them though because sometimes it takes 5 paragraph to explain the answer to something that they thought was a yes or no answer.
Pick a thread title that says what the thread is about - always.

First things first, my bad iMalc hence forth I shall post thread titles that are proper, my apologies. Thank you to everyone who commented, I appreciate the help and criticism, I have some re-writing to do, but this will have to wait until after work!

Sorry, I was trying not to pick on you.
It's just one eventually reaches the point where it feels like it's time to say something.

Oh and hey, feel free to post updated code if you'd like to confirm that you've understood the advice people gave, and see if anyone has any further tips.

No worries iMalc! Well everyone, here is my code after taking suggestions into account... Please let me know what you think and if I can improve it anywhere. Also, I did leave the system call in there simply because its the most convenient for now, I looked at the page smokeyangel referenced and will probably end up grabbing the curses library eventually.

Code:

#include <iostream>
#include <stdlib.h>
#include <ctime>


using namespace std;


int randNum (int low, int high)
{
    return rand() % (high-low) + low;
}


int main()
{
    srand (time(NULL));
    int num0;
    int num1;
    int num2;


    for (int i = 0; i < 250; ++i)
    {
        num0 = randNum(1, 10);
        num1 = randNum(1, 10);
        num2 = randNum(1, 10);
        system("CLS");
        cout << "-------" <<endl;
        cout << "|" << num0 << "|" << num1 << "|" << num2 << "|\n";
        cout << "-------" <<endl;
    }


    if (num0 == num1 && num1 == num2)
    {
        cout << "You won " <<num0 <<"dollars. Congratulations!\n";
    }
}

Thank you all for your help and advice!

I see that you must have noticed that your random number function was only producing numbers from 1 to 8, and so bumped the second parameter to 10.
The other way to fix that would have been to use "(high-low+1)" inside the randNum function. But either way works just as good. C# requires one-greater-than-the-max like you have.

Originally Posted by rTeapot

I wish switches could take in multiple variable dependencies but alas they cannot :/.

Although you don't need it for the cases you have so far, it's simple to combine several variables
into one value for a switch statement.

Like this:

Code:

    switch (num0*100 + num1*10 + num2)
    {   case 112:
            . . . ;
            break;

        case 133:
            . . . ;
            break;

        case 644:
            . . . ;
            break;
    }

Notice that the values for num0, num1, and num2 are all obvious in the cases constants,
1 1 2
1 3 3
etc.

One alternative to system() that you may want to consider is printing "\r". This is a "carriage return", which moves the cursor to the beginning of the line but not down to the next line ("\n" does both of these). This allows you to overwrite something you've already written, as long as it's on the same line. You may find that you'll have to flush the output (cout << flush) to get the text visible; and you'll probably also find that it's so fast now that you need another delay because the user can't see the different lines you're printing out!

"\r" is perfectly portable, as far as I know (though some systems might not do precisely what you want with the character). Figuring out how to portably insert delays might be a challenge though...