Mystery bug in simple program? - I am beginner

I am just beginning to write C++ and have come accross a bug in a simple MS-DOS program. The program is a basic calculator. when it is run and a non-numeric entry is entered into a 'float' field, the text goes haywire and keeps on repeating itself.
Please, Please, Please will somebody help me as this is my first real program. Thank You, Archie

Here is the program, created using Bloodshed Dev-C++:

Code:

---

#include <iostream>
using namespace std;
int main()
{
    for ( float x = 0; x < 1; x + 0 ) {
    float number;
    cout<<"CALCULATOR\nArchie Lodge\n\nTo multiply, type '1'\nTo divide, type '2'\nTo add, type '3'\nTo subtract, type '4'\nType number and then press Enter...";
    cin>> number;
  cin.ignore();
  if ( number == 1 ) {//MULTIPLY
    float thisisanumber;
    float thisisnumbertwo;
    cout<<"Please enter the first number to be multiplied: ";
    cin>> thisisanumber;
    cin.ignore();
    cout<<"Please enter the number to multiply the above number by: ";
    cin>> thisisnumbertwo;
    cin.ignore();
    cout<<"The numbers you entered, multiplied together are: "<< thisisanumber * thisisnumbertwo<<"\n";
    cout<< thisisanumber <<" times " <<thisisnumbertwo<<" is "<< thisisanumber * thisisnumbertwo<<"\n\nPlease press enter twice to perform another calculation.\n";
    cin.get();
}
  else if ( number == 2 ) {//DIVIDE
    float thisisanumber;
    float thisisnumbertwo;
    cout<<"Please enter the first number to be divided: ";
    cin>> thisisanumber;
    cin.ignore();
    cout<<"Please enter the number for the above number to be divided by: ";
    cin>> thisisnumbertwo;
    cin.ignore();
    cout<<"The numbers you entered, divided are: "<< thisisanumber / thisisnumbertwo<<"\n";
    cout<< thisisanumber <<" divided by " <<thisisnumbertwo<<" is "<< thisisanumber / thisisnumbertwo<<"\n\nPlease press enter twice to perform another calculation.\n";
    cin.get();
}
else if ( number == 3 ) {//ADD
    float thisisanumber;
    float thisisnumbertwo;
    cout<<"Please enter the first number to be added: ";
    cin>> thisisanumber;
    cin.ignore();
    cout<<"Please enter the number to add to the above number: ";
    cin>> thisisnumbertwo;
    cin.ignore();
    cout<<"The numbers you entered, added together are: "<< thisisanumber + thisisnumbertwo<<"\n";
    cout<< thisisanumber <<" plus " <<thisisnumbertwo<<" is "<< thisisanumber + thisisnumbertwo<<"\n\nPlease press enter twice to perform another calculation.\n";
    cin.get();
}
else if ( number == 4 ) {//SUBTRACT
    float thisisanumber;
    float thisisnumbertwo;
    cout<<"Please enter the starting number: ";
    cin>> thisisanumber;
    cin.ignore();
    cout<<"Please enter the number to be subtracted from the above number: ";
    cin>> thisisnumbertwo;
    cin.ignore();
    cout<<"The first number you entered, minus the second number are: "<< thisisanumber - thisisnumbertwo<<"\n";
    cout<< thisisanumber <<" minus " <<thisisnumbertwo<<" is "<< thisisanumber - thisisnumbertwo<<"\n\nPlease press enter twice to perform another calculation.\n";
    cin.get();
}
  else {//ERROR
    cout<<"Please enter either 1, 2, 3 or 4. Press enter to continue.\n";
  }
  cin.get();
}
}

---

Well, that's simple enough. Don't enter anything other than a number when asking for a number ;)

Seriously, though, if you want to make sure that doesn't happen, you can read in the input as a string and then parse that to get your float. That way, if the user decides to enter their favorite color, you can throw an error and tell the user to enter a number.

Couple of tips:

'thisisanumber' and 'thisisnumber2' are declared more than once. You don't need that. Also, it'd be much neater if you separated your tasks into user-defined functions and indented your code right. Also, switch/case statements are better to use than a bunch of else ifs.

And don't forget to return 0.

....Also, your program won't allow someone to "press Enter twice to do another calculation" because you're not using any kind of loop to bring it back.

Quote:

---

Originally Posted by Krak

'thisisanumber' and 'thisisnumber2' are declared more than once.

---

Not exactly. Those variables are declared within each branch of the program, so what he's doing isn't illegal since each thisisanumber is in a different block. However, it would be best, I think, to just declare those variables at the beginning of main().

When you enter non-numeric input where numeric input is expected, that non-numeric input sets the input stream into a bad state and the data doesn't get extracted from the stream. All attempts to read from the stream will automatically fail until you reset the stream's error flags and clear out the bad data. Only then will you be able to continue to process input as normal.

To make this easy, it might make sense to create a function to wrap all this code:

Code:

---

#include <limits>

...

bool GotAFloat(char* msg,float& f)
{
    cout << msg;
    if( !(cin >> f) )
    {
        // Clear error flags
        cin.clear();
        // Pass over any bad data in the stream
        cin.ignore(numeric_limits<streamsize>::max(),'\n');
        return false;
    }
    return true;
}

---

And then instead of calling the stream extractor directly in your code whenever you want to get a float, you would instead call the function:

Code:

---

//Instead of following
cout<<"Please enter the first number to be multiplied: ";
cin>> thisisanumber;
cin.ignore();

// Do this instead
while( !GotAFloat("Please enter the first number to be multiplied:",thisisanumber) );

---

Code:

---

for ( float x = 0; x < 1; x + 0 ) {

---

An easier way to do that is:

Code:

---

while(1) {

---

You should probably also have an option 5 to exit the program as well that would call break to exit the loop and the program.

I have reieved a reccomendation from hk_mp5kpdw to put a block of code into my program. I have fixed all of the other issues brought up. Thank You Everybody!!! :)

This is the confusing piece of code:

Quote:

---

Originally Posted by hk_mp5kpdw

To make this easy, it might make sense to create a function to wrap all this code:

Code:

---

#include <limits> //I know that this is at the top.

...

bool GotAFloat(char* msg,float& f)  //But where does this go?
{
    cout << msg;
    if( !(cin >> f) )
    {
        // Clear error flags
        cin.clear();
        // Pass over any bad data in the stream
        cin.ignore(numeric_limits<streamsize>::max(),'\n');
        return false;
    }
    return true
}

---

And then instead of calling the stream extractor directly in your code whenever you want to get a float, you would instead call the function:

Code:

---

//Instead of following
cout<<"Please enter the first number to be multiplied: ";
cin>> thisisanumber;
cin.ignore();

// Do this instead
while( !GotAFloat("Please enter the first number to be multiplied:",thisisanumber) );

---

Code:

---

for ( float x = 0; x < 1; x + 0 ) {

---

An easier way to do that is:

Code:

---

while(1) {

---

You should probably also have an option 5 to exit the program as well that would call break to exit the loop and the program.

---

In your case I would think that you could put the GotAFloat function in between the using namespace std; line and the int main() line. You could put it after the end of the main function but then you would need to prototype it.

Thank you so much... It works! :D :D :D