Newbie'z first real program. Could use some CONSTRUCTIVE critizism to learn...

[Jeff++]

Please keep in mind I only started programming 9 days ago. Before then all I knew was HTML and a touch of PHP ( not much ).

I would love it if some of you pro's out there could take a look at this monster I've created over the last 2 days, and tell me if I could have, or should have done something different than what I've done.

I Created a game called... " Mr. Hell's IQ "
Mr. Hell is a guy on a message board out there in cyberspace who spends ALL DAY ripping on my band. We think it's hallarious. Some of the things he says are so increadibly stupid that we can't help but roll on the floor with laughter.
This game is just for a laugh. You need to guess Mr. Hell's IQ , then go from there.
it's fun.

Please let me know if you think I could have done the code another way, or better way.

Thanks everyone.

#include <iostream>
#include <windows.h>
using namespace std;
int iq;
int iq2;
int calc;
int y = 1;
int counter = 0;
int main()
{
    cout << " Welcome to the : Guess Mr. Hell's IQ Game !!\n\n";
    cout << " To Win You Will Have to follow these simple rules:\n\n * guess Mr. Hell's IQ in less than 10 tries,\n * Complete the questions asked after you get it,\n * You can't say no to ANY questions.\n";
    cout << "\n Once You guess Mr. Hell's IQ,\n you will have to PROVE you're smarter than Mr. Hell\n";
    cout << "\n\n What's Your First Guess ?\n\n";


    int maxrand = 40;
    srand( (unsigned)time( NULL ) );
    int iqanswer = rand() &#37; maxrand;

    while(iq != maxrand, ++counter)
    {

    cin>>iq;
    if(iq == iqanswer)cout << "\n Right On !  You guessed it. He Really is THAT stupid !!\n\n";
    if(iq == iqanswer) break;
    if(iq > iqanswer)cout <<"\n Too High\n\n Try Again: ";
    if(iq < iqanswer)cout << "\n Too Low\n\n Try Again: ";
    if(counter == 10)
    {
         cout << "\n\n You're Not Too Bright Yourself Now Are You ?\n Are You Sure You're Not Mr. Hell ?\n\n It Was: "; cout<< iqanswer; cout << " Dumb-*** !\n\n";
     break;
     }
    }
    cout << " Now You Will Have To Guess George Bush's IQ\n\n Go Ahead and take a Wild Guess: ";

    counter = 0;

    int maxrand2 = 50;
    srand( (unsigned)time( NULL ) ); 
    int iqanswer2 = rand() % maxrand2;

    while( iq2 != maxrand2, ++counter){
    cin>>iq2;
    if(iq2 == iqanswer2)cout << "\n Nice Work !  You guessed it. He is also pretty Dumb huh ?\n\n\n\n";
    if(iq2 == iqanswer2) break;
    if(iq2 > iqanswer2)cout <<"\n Too High\n\nTry Again: ";
    if(iq2 < iqanswer2)cout << "\n Too Low\n\nTry Again: ";
    if(counter == 10)
    {
         cout << "\n\n You're Not Too Bright Yourself Now Are You ?\n Are You Sure You're Not Mr. Hell ?\n\nIt Was: ";
         cout << iqanswer2; cout << " Dumb-*** !\n\n";
             break;
     }
   }

   cout << " Now You Will Need To Add The Two IQ's Together....\n\n";
   cout << " If You Are As Dumb As Mr. Hell You May Need A Calculator...\n\n";
   cout << " Would You like to use a Calculator ?\n\n press  1  for the calculator,\n press any other key for no if you think you don't need it.\n\n";


   {
    cin >> calc;
     if(calc == y)
  {

    ::ShellExecute(NULL,
                   "open",
                   "calc.exe",
                   "",
                   "",
                   SW_SHOWNORMAL);

    }    
    else
    {
        cout << "\n\n Ha Ha You're as Dumb As Mr. Hell !!\n You Loose Sucker....\n\n You could NOT answer NO to ANY question REMEMBER ???";
    }

    if(calc == y )
    {
    int total;
    int totalanswer = (iqanswer + iqanswer2);

    cout << " Now add Mr. Hell's IQ to George Bush's IQ and type your answer here: ";
    cin >> total;



    if(total == totalanswer) { cout << "\n\n YOU WIN !!!\n\n Very Good !\n\n You Clearly are a step ahead of Mr. Hell, and your IQ is higher than Both\n Mr. Hell's and George Bush's IQ COMBINED !!\n\n\n\n";

    }

    else
    {
        cout <<  "\n\n Ha Ha !\n\n You lack of adding skills suggests\n You are as Dumb as Mr. Hell !\n\nThe Correct Answer was: "; cout << totalanswer;  cout << "\n\n\n\n";
    }



    }

    cout << "\n\n   GAME OVER\n\n  Press any key and Enter to close.";
    scanf("%");
    return 0;
   }
}

All critizism welcome !

cheers.

[Peter_APIIT]

if(counter == 10)
    {
         cout << "\n\n You're Not Too Bright Yourself Now Are You ?\n Are You Sure You're Not Mr. Hell ?\n\n It Was: "; cout<< iqanswer; cout << " Dumb-*** !\n\n";
     break;
     }

Use a boolean variable to exit from loop.

[Jeff++]

Hey Peter,
thanks for the comments.

can you tell me why I shouldn't use break; here ?

and maybe give me an example of what you mean ?

thanks.

: )

[GCDEF]

You should only call srand once.

You really need to get out of the habit of putting multiple statements on one line.

iq is uninitialized when you first use it in your while loop.

These

   if(iq == iqanswer)cout << "\n Right On !  You guessed it. He Really is THAT stupid !!\n\n";
    if(iq == iqanswer) break;

should be consolidated into one

if(iq == iqanswer)
{
      cout << "\n Right On !  You guessed it. He Really is THAT stupid !!\n\n";
      break;
}

I really doubt this is doing what you think

    cin >> calc;
     if(calc == y)

[Jeff++]

hey GCDEF,
thanks for the tips. I see what you're saying about using { } I have been doing that already with my new program. I will have to change that in this one.


if I should only call srand once, then how whould I get a new random number for the second function, and make the first one will reset when played again ? without it, it stays on the same number everytime you play the game.


what do you mean iq isn't initialized in the while loop ?

and the cin >> calc; is asking for user input and stores it in the variable named calc. if calc is equal to 1 ( y ) then it continues with the open calculator function. That's what it does, so.... what do you mean ? Shoud I have done this another way ?


thanks again.

[potatoCode]

Hello Jeff++

if I should only call srand once, then how whould I get a new random number for the second function, and make the first one will reset when played again ? without it, it stays on the same number everytime you play the game.

this might help.

what do you mean iq isn't initialized in the while loop ?

GCDEF is pointing to

int iq; // always good practice to initialize your variables
/* same as before */
while(iq != maxrand, ++counter) // here iq is uninitialized

and the cin >> calc; is asking for user input and stores it in the variable named calc. if calc is equal to 1 ( y ) then it continues with the open calculator function. That's what it does, so.... what do you mean ? Shoud I have done this another way ?

I think he means the algorithm, not the actual functionality of the code.

And yeah haha, I laughed when I got the joke. A good one!

[Jeff++]

hey thanks potatocode..

I have int iq; at the beginning of the program, so you mean I have to delcare it again ?
or should I NOT have put it at the beginning of the program ?

thanks for the link to the article on the use of rand(); I get the problem with seeding rand with system time.... and perhaps that is exactly what was happening to me when I first put the rand function in there.... the first random number was always the same when I re-started the program... but perhaps that's because I was starting it several times within 1 minute. Once I added the

 srand( (unsigned)time( NULL ) );

to initialize the random number it changed the random number perfectly, nice and fast.
is this still a no no ?

thanks.

[JohnW@Wessex]

I have int iq; at the beginning of the program, so you mean I have to delcare it again ?
or should I NOT have put it at the beginning of the program ?

#include <iostream>
#include <windows.h>
using namespace std;
int iq;
int iq2;
int calc;
int y = 1;
int counter = 0;
int main()
{
    cout << " Welcome to the : Guess Mr. Hell's IQ Game !!\n\n";
    cout << " To Win You Will Have to follow these simple rules:\n\n * guess Mr. Hell's IQ in less than 10 tries,\n * Complete the questions asked after you get it,\n * You can't say no to ANY questions.\n";
    cout << "\n Once You guess Mr. Hell's IQ,\n you will have to PROVE you're smarter than Mr. Hell\n";
    cout << "\n\n What's Your First Guess ?\n\n";


    int maxrand = 40;
    srand( (unsigned)time( NULL ) );
    int iqanswer = rand() % maxrand;

    while(iq != maxrand, ++counter)

Where is iq initialised?

[Jeff++]

so you mean it should be

#include <iostream>
#include <windows.h>
using namespace std;
int iq = 0;  //  Is this what you mean ??  
int iq2 = 0;
int calc = 0;
int y = 1;
int counter = 0;
int main()
{
    cout << " Welcome to the : Guess Mr. Hell's IQ Game !!\n\n";
    cout << " To Win You Will Have to follow these simple rules:\n\n * guess Mr. Hell's IQ in less than 10 tries,\n * Complete the questions asked after you get it,\n * You can't say no to ANY questions.\n";
    cout << "\n Once You guess Mr. Hell's IQ,\n you will have to PROVE you're smarter than Mr. Hell\n";
    cout << "\n\n What's Your First Guess ?\n\n";


    int maxrand = 40;
    srand( (unsigned)time( NULL ) );
    int iqanswer = rand() % maxrand;

    while(iq != maxrand, ++counter)

[potatoCode]

Hello Jeff++,
although I enjoyed your game (a good twist!),
I'm afraid you're missing out on some basics. For built-in types, C++ offers two types of initialization

int iq(25);  // this is called direct initialization
int iq = 25;  // this is called copy initialization

Don't worry about the difference for now, but suffice it to say that you must initialize the variable before it is used regardless of where it appears in your source code.

Because I myself am a newbie, I still remember all too well how things could get so confusing. And I found it that learning from the book (not from the online resources) step by step saved time and headache in learning. That is of course, you're teaching yourself.

Hope this helps
bye~

[Jeff++]

so should I use the iq = 0; or the iq(0); ????

[Jeff++]

I have re-built the game with a new twist....

I have made the suggested changes for better coding, ( thanks everyone )

but I also though... hey... Mr. Hell really hates my band a lot.. so when I send him a link to my game.... why not have a special treat for him at the end ??

something he would really love ???

I also made the spacing a little better on the screen for the words at the end of the game.

I also cut down the guesses for IQ 's to 8 from 10. Don't want it to be TOOOOOO easy now !

have fun playing. Let me know what else I could do better
and .....


can someone PLEASE tell me why I can't get cin.get(); to work at the end of the program !!!
thanks everyone. : )

#include <iostream>
#include <windows.h>
using namespace std;
int iq = 0;
int iq2 = 0;
int calc = 0;
int y = 1;
int counter = 0;
int main()
{
    cout << " Welcome to the : Guess Mr. Hell's IQ Game !!\n\n";
    cout << " To Win You Will Have to follow these simple rules:\n\n * guess Mr. Hell's IQ in less than 8 tries,\n * Complete the questions asked after you get it,\n * You can't say no to ANY questions.\n";
    cout << "\n Once You guess Mr. Hell's IQ,\n you will have to PROVE you're smarter than Mr. Hell\n";
    cout << "\n\n What's Your First Guess ?\n\n";


    int maxrand = 40;
    srand( (unsigned)time( NULL ) );
    int iqanswer = rand() &#37; maxrand;

    while(iq != maxrand, ++counter)
    {

    cin>>iq;
    if(iq == iqanswer)
    {
    cout << "\n Right On !  You guessed it. He Really is THAT stupid !!\n\n";
    break;
    }
    if(iq > iqanswer)cout <<"\n Too High\n\n Try Again: ";
    if(iq < iqanswer)cout << "\n Too Low\n\n Try Again: ";
    if(counter == 8)
    {
    cout << "\n\n You're Not Too Bright Yourself Now Are You ?\n Are You Sure You're Not Mr. Hell ?\n\n It Was: ";
    cout<< iqanswer; cout << " Dumb-*** !\n\n";
     break;
     }
    }
    cout << " Now You Will Have To Guess George Bush's IQ\n\n Go Ahead and take a Wild Guess: ";

    counter = 0;

    int maxrand2 = 50;
    srand( (unsigned)time( NULL ) );
    int iqanswer2 = rand() % maxrand2;

    while( iq2 != maxrand2, ++counter){
    cin>>iq2;
    if(iq2 == iqanswer2)
    {
    cout << "\n Nice Work !  You guessed it. He is also pretty Dumb huh ?\n\n\n\n";
    break;
    }
    if(iq2 > iqanswer2)cout <<"\n Too High\n\nTry Again: ";
    if(iq2 < iqanswer2)cout << "\n Too Low\n\nTry Again: ";
    if(counter == 8)
    {
         cout << "\n\n You're Not Too Bright Yourself Now Are You ?\n Are You Sure You're Not Mr. Hell ?\n\nIt Was: ";
         cout << iqanswer2; cout << " Dumb-*** !\n\n";
             break;
     }
   }

   cout << " Now You Will Need To Add The Two IQ's Together....\n\n";
   cout << " If You Are As Dumb As Mr. Hell You May Need A Calculator...\n\n";
   cout << " Would You like to use a Calculator ?\n\n press  1  for the calculator,\n press any other key for no if you think you don't need it.\n\n";


   {
    cin >> calc;
     if(calc == y)
  {

    ::ShellExecute(NULL,
                   "open",
                   "calc.exe",
                   "",
                   "",
                   SW_SHOWNORMAL);

    }
    else
    {
        cout << "\n\n\n\n\n\n\n\n\n\n Ha Ha You're as Dumb As Mr. Hell !!\n You Loose Sucker....\n\n You could NOT answer NO to ANY question REMEMBER ???";
    }

    if(calc == y )
    {
    int total;
    int totalanswer = (iqanswer + iqanswer2);

    cout << " Now add Mr. Hell's IQ to George Bush's IQ and type your answer here: ";
    cin >> total;



    if(total == totalanswer) { cout << "\n\n\n YOU WIN !!!\n\n Very Good !\n\n You Clearly are a step ahead of Mr. Hell, and your IQ is higher than Both\n Mr. Hell's and George Bush's IQ COMBINED !!";

    }

    else
    {
        cout <<  "\n\n  *  GAME OVER   * Ha Ha !\n\n You lack of adding skills suggests\n You are as Dumb as Mr. Hell !\n\nThe Correct Answer was: ";
        cout << totalanswer;
    }

    }


        cout<< "\n\n Thanks for Playing.\n\n You Are Now Worthy Of A Special Treat From\n\n The Worlds Greatest New Alien Rock Band...\n\n\n    T H E   P E R I S H   !! \n\n\n\n    press any key, then ENTER to continue... \n\n\n\n\n";
        scanf("%"); // if I use cin.get();  here instead it does NOT pause and wait for a key press. WHY ??????

    {

  ::ShellExecute(NULL,
                   "open",
                   "http://www.youtube.com/watch?v=TwazkWbE4Pk&feature=channel_page",
                   "",
                   "",
                   SW_SHOWNORMAL);


    scanf("%");// if I use cin.get();  here instead it does NOT pause and wait for a key press. WHY ??????

}

   }

}

[Jeff++]

here's a link to the zip file with the game as an exe with cute little Devil icon if anyone wants to share it.
cheers.

http://theperish.ca/The%20Mr.%20Hell%20Game.zip

[GCDEF]

Better, but you still have multiple statements per line and you're still calling srand twice. You'll find that style of coding much harder to read and debug. Maybe not a big deal now, but when your programs involve hundreds of thousands of lines of code, you want them as readable, debuggable an maintainable as you can get them.

You should call srand once and subsequent calls to rand generate a different number.

Also, int maxrand = 40; isn't the way you go about defining constants. I know what you're going for, but the standard way would be to put
#define maxrand 40
after your include statements or in a header file.

[JohnW@Wessex]

Or
const int maxrand = 40;