Problem with my code

[Mitosis]

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

using namespace std;

int main(int argc, char *argv[])
{
 cout << "Welcome to Jon's Scientific Calculator." << endl;
 cout << "1 : Addition" << endl;
 cout << "2 : Subtract" << endl;
 cout << "3 : Multiplication" << endl;
 cout << "4 : Division" << endl;
 int q;
 int w;
 int e;
 int r;
 int x;
 int y;
 cin >> q;
 cin >> w;
 cin >> e;
 cin >> r;
 if (q ==1)
 cout << "Addition" << endl;
 cin >> x;
 cin >> y;
 cout << x + y << endl;

 if  (w ==2)
 cout << "Subtraction" << endl;
 cin >> x;
 cin >> y;
 cout << x - y << endl;

 if (e == 3)
 cout << "Multiplication" << endl;
 cin >> x;
 cin >> y;
 cout << x * y << endl;

 if (r ==4)
 cout << "Division" << endl;
 cin >> x;
 cin >> y;
 cout << x / y << endl;
 system("PAUSE");   
 return 0;
}

Now when I run it, when I press the number that I want it wont display the information held. It will continue to act like you need to press a number in for 4 more times. Its wierd...I think I did something wrong with my code but Im not quite to sure. If anyone could help me figure it you, thanks.

[Moonshine]

Firstly, it's spelled "problem"

But seriously, you need curly braces {} around the if() statements when you want to execute more than one line of code underneath them.

And also, you only need one variable to get the menu choice, so instead of
int q,w,e,r and all of that, just put:

int choice;
cin >> choice;

if (choice == 1) {

... stuff here
}

if (choice == 2) {

... stuff here
}

etc

Hope this helps.

[Mephisto]

I'm not trying to discourage you from learning C, but I just have to say your style is awful.  If you're going to declare a list of variables of the same type either put them into an array or declare them in one statement using the comma (,) operator.  Also, your calculator needs more functionality.  I recommend just making a menu, have them select what operation they want, then ask them for their integers, floats, etc. (a static cast comes to mind), then perform the calculation, give them their answer, offer them the option to perform another calculation or exit, and you should have a good calculator.  I would later add more functionality to it later.  Considering looking at math.h and using logic to create your own functions.

Edit:  Here's a calculator program I put together -- if you have questions just ask me on this thread, though you should understand this program fully with my notes and just compiling/running/debugging and simply reading the source code...

Code Select Expand


// this program contains recursion for main() so do not compile this as a Win32 Program -- this is designed for console
// this program also uses windows.h for the Sleep() function which pauses the program for a given amount of milliseconds
// this program also uses conio.h for the getch() function which essentially is input w/o a cursor
// the modulus is something you may have not heard in math, but it will return the remainder of the division
// this program also does not support decimal numbers (floats) -- if you want this implementation tell me and I be glad to help you
#include <iostream>
#include <conio.h>
#include <stdlib.h>
#include <windows.h>
using std::cout;
using std::cin;
using std::endl;
// you could just use the statement "using namespace std" but that's bad programming practice if you're only using a few objects from the std namespace
int main() {
   int variables[5]; // 0 = operator choice, 1 = first digit, 2 = second digit, 3 = last option, 4 = exit
   cout << "Welcome to Jon's Scientific Calculator!\n" << endl;
   cout << "Select your mathematical operation...\n";
   cout << "[1] - Addition\n";
   cout << "[2] - Subtraction\n";
   cout << "[3] - Multipalcation\n";
   cout << "[4] - Division\n";
   cout << "[5] - Modulus\n";
   cin >> variables[0];
   switch(variables[0]) {
       case 1:
           cout << "You have chosen the addition operation...\n\n";
           cout << "Please enter your first digit: ";
           cin >> variables[1];
           cout << "\n";
           cout << "Please enter your second digit: ";
           cin >> variables[2];
           cout << "\n\n";
           cout << variables[1] + variables[2] << endl;
           cout << "\n\n";
           break;
       case 2:
           cout << "You have chosen the subtraction operation...\n\n";
           cout << "Please enter your first digit: ";
           cin >> variables[1];
           cout << "\n";
           cout << "Please enter your second digit: ";
           cin >> variables[2];
           cout << "\n\n";
           cout << variables[1] - variables[2] << endl;
           cout << "\n\n";
           break;
       case 3:
           cout << "You have chosen the multipalcation operation...\n\n";
           cout << "Please enter your first digit: ";
           cin >> variables[1];
           cout << "\n";
           cout << "Please enter your second digit: ";
           cin >> variables[2];
           cout << "\n\n";
           cout << variables[1] * variables[2] << endl;
           cout << "\n\n";
           break;
       case 4:
           cout << "You have chosen the division operation...\n\n";
           cout << "Please enter your first digit: ";
           cin >> variables[1];
           cout << "\n";
           cout << "Please enter your second digit: ";
           cin >> variables[2];
           cout << "\n\n";
           cout << variables[1] / variables[2] << endl;
           cout << "\n\n";
           break;
       case 5:
           cout << "You have chosen the modulus operation...\n\n";
           cout << "Please enter your first digit: ";
           cin >> variables[1];
           cout << "\n";
           cout << "Please enter your second digit: ";
           cin >> variables[2];
           cout << "\n\n";
           cout << variables[1] % variables[2] << endl;
           cout << "\n\n";
           break;
       default:
           cout << "Bad input option...restarting...\n";
           Sleep(2000); // so they have time to read the error message
           system("cls");
           main();
           cout << "\n\n";
           break;
   }
   cout << "Pleae choose...\n";
   cout << "[1] - calculate a new operation\n";
   cout << "[2] - exit the program...\n";
   cin >> variables[3];
   if (variables[3] == 1) {
       system("cls");
       main();
   }
   else if (variables[3] == 2)
       return 0;
   else
       return 0;
   system("cls");
   cout << "Ending calculator...\n";
   variables[4] = getch();
   return 0;
}


[Eli_1]

if (variables[3] == 1) {
   system("cls");
   main(); // <----
}

Eww.

[Mephisto]

I don't see anything bad about calling main() again in a console app.  I was about to do what I would normally do and that's to use a goto statement, but I figured I get some nasty personal opinionated remarks...Also, I didn't want to break the program into a class or functions because I'm not too sure if this guy knows about those yet.  Correct me if I'm wrong.  :p

[iago]

If you are inputting 2 variables, I wouldn't use an array.  If you aren't looping through them, or using the array functionality at all, why use an array?

Also, I hate declaring more than one variable on a line with a comma, I will actively discourage that.  Although you can do it, I prefer not.

Third, you're inputting the two variables (variables[1] and variables[2] for some reason, but like I already said putting them in an array is silly) 5 different times.  Why not do it before the switch?

Finally, I've been told you shouldn't cout << "\n", endl has extra functionality (flushing the buffer) that should be used.

And double-finally, calling main is ewww.  Recursion is bad.  Every time it runs it adds new stuff to the stack.  Depending on the OS, sometimes a lot of stuff is added.  I would use a do..while loop for this.  Or, better yet, put everything into a function and have a do..while in main that calls that function.  <edit> but you said why you didn't do a function, which is fine.

Triple finally:

else if (variables[3] == 2)
       return 0;
   else
       return 0;
   system("cls");
   cout << "Ending calculator...\n";
   variables[4] = getch();
   return 0;

Why do you need to end main 3 times there?  Why can't that all be the same?  

And why are you inputting a dummy value into an array?

And as soon as you do system("cls") you're making it windows specifiic - Linux uses clear.  Though I don't see why you'd want to clear the screen anyway.

[Mephisto]

Because it looks disgusting without clearing the screen.  Also, I'm making it Windows specific with Sleep() aswell, and I don't ever use Linux so it's not really my responsibility to bother with making sure it's Linux compatable.  Also, I'm sure he's using Windows.  

Good idea though, I completely forgot about a while loop when I was thinking about it...I assume you mean using continue/break statements throughout the loop when they want to repeat an operation of exit?

[Eli_1]

Also, I'm making it Windows specific with Sleep() aswell.

What about iostream.h?

Code Select Expand


cout << "Bad input option...restarting...\n";
           Sleep(2000); // so they have time to read the error message

Why not use getchr();?

Linux uses clear

Does Linux have conio.h? And if it does, isn't there a clrscr();?

[iago]

Also, I'm making it Windows specific with Sleep() aswell.

What about iostream.h?

Code Select Expand


cout << "Bad input option...restarting...\n";
           Sleep(2000); // so they have time to read the error message

Why not use getchr();?

Linux uses clear

Does Linux have conio.h? And if it does, isn't there a clrscr();?

Linux has iostream.

sleep(2000) is fine, for a pause, but I wouldn't pause or clear.

[Eli_1]

Oh, I didn't think Linux had iostream. That was why I switched to stdio.h, which I'm much happier with anyway.

[MyndFyre]

I don't see anything bad about calling main() again in a console app.  I was about to do what I would normally do and that's to use a goto statement, but I figured I get some nasty personal opinionated remarks...Also, I didn't want to break the program into a class or functions because I'm not too sure if this guy knows about those yet.  Correct me if I'm wrong.  :p

OK, I stopped reading right here, so if someone already touched on this, I'm sorry.  The other posts were "eh," but this one definitely required attention.

The problem with calling the "main()" mutliple times within the same program is the overutilization of the call stack.  Say that you want to run a program that automatically sends input to your program; maybe after 1000 test cases, your program will crash with an out-of-memory error, possibly causing a huge memory leak if you weren't equipped to deal with the error properly.

Rather, I would do something like this (actually reminiscint of the honors project loop I did in my first Java class @ ASU):

Code Select Expand


#define OPT_EXIT 'e'
#define OPT_CONTINUE 'c'
#define OPT_ADD 'a'
#define OPT_SUB 's'
#define OPT_MUL 'm'
#define OPT_DIV 'd'
#define OPT_REM 'r'
// that's the remainder -- modulus

int main()
{
 char opt = OPT_CONTINUE;
 do
 {
    displayMenu();
    switch (displayPrompt())
   {
      case OPT_ADD:
        // do addition stuff
       break;
      case OPT_SUB:
        // do addition stuff
       break;
      case OPT_MUL:
        // do addition stuff
       break;
      case OPT_DIV:
        // do addition stuff
       break;
      case OPT_REM:
        // do addition stuff
       break;
      default:
        cout << "You did not specify a valid option.";
        break;
   }

   cout << "Would you like to go again?";
   displayContinueExit(&opt);
 } while (opt == OPT_CONTINUE);
}


The elegance of the do...while loop is that:
1.) The code is always executed at least once.
2.) It's a simple jump, not affecting the call stack.
3.) It's easier to debug than recursion.
4.) You can clean up local variables at the end of the function without worrying about screwing up those variables for higher-level callees.

Cheers.

[Yoni]

Oh, I didn't think Linux had iostream. That was why I switched to stdio.h, which I'm much happier with anyway.

iostream is standard C++. Any good implementation of C++ on any platform has it.

[iago]

Code Select Expand


 if (variables[3] == 1) {
       system("cls");
       main();
   }
   else if (variables[3] == 2)
       return 0;
   else
       return 0;
   system("cls");
   cout << "Ending calculator...\n";
   variables[4] = getch();
   return 0;
}


I officially withdraw my complaits about using system("cls") here.  Because you have "else return 0;", system("cls") can't possibly get executed anyway, so it's not a problem

And instead of variables[4] = getch();, why don't you just do getch()?  But again, that can't possibly run, so it's ok.

[Mitosis]

Its alright..

Anyway I saw your code that you did a while ago and had a couple questions.

#include <iostream>
#include <conio.h>
#include <stdlib.h>
#include <windows.h>
using std::cout;
using std::cin;
using std::endl;
// you could just use the statement "using namespace std" but that's bad programming practice if you're only using a few objects from the std namespace
int main() {
   int variables[5]; // 0 = operator choice, 1 = first digit, 2 = second digit, 3 = last option, 4 = exit
   cout << "Welcome to Jon's Scientific Calculator!\n" << endl;
   cout << "Select your mathematical operation...\n";
   cout << "[1] - Addition\n";
   cout << "[2] - Subtraction\n";
   cout << "[3] - Multipalcation\n";
   cout << "[4] - Division\n";
   cout << "[5] - Modulus\n";
   cin >> variables[0];
   switch(variables[0]) {
       case 1:
           cout << "You have chosen the addition operation...\n\n";
           cout << "Please enter your first digit: ";
           cin >> variables[1];
           cout << "\n";
           cout << "Please enter your second digit: ";
           cin >> variables[2];
           cout << "\n\n";
           cout << variables[1] + variables[2] << endl;
           cout << "\n\n";
           break;
       case 2:
           cout << "You have chosen the subtraction operation...\n\n";
           cout << "Please enter your first digit: ";
           cin >> variables[1];
           cout << "\n";
           cout << "Please enter your second digit: ";
           cin >> variables[2];
           cout << "\n\n";
           cout << variables[1] - variables[2] << endl;
           cout << "\n\n";
           break;
       case 3:
           cout << "You have chosen the multipalcation operation...\n\n";
           cout << "Please enter your first digit: ";
           cin >> variables[1];
           cout << "\n";
           cout << "Please enter your second digit: ";
           cin >> variables[2];
           cout << "\n\n";
           cout << variables[1] * variables[2] << endl;
           cout << "\n\n";
           break;
       case 4:
           cout << "You have chosen the division operation...\n\n";
           cout << "Please enter your first digit: ";
           cin >> variables[1];
           cout << "\n";
           cout << "Please enter your second digit: ";
           cin >> variables[2];
           cout << "\n\n";
           cout << variables[1] / variables[2] << endl;
           cout << "\n\n";
           break;
       case 5:
           cout << "You have chosen the modulus operation...\n\n";
           cout << "Please enter your first digit: ";
           cin >> variables[1];
           cout << "\n";
           cout << "Please enter your second digit: ";
           cin >> variables[2];
           cout << "\n\n";
           cout << variables[1] % variables[2] << endl;
           cout << "\n\n";
           break;
       default:
           cout << "Bad input option...restarting...\n";
           Sleep(2000); // so they have time to read the error message
           system("cls");
           main();
           cout << "\n\n";
           break;
   }
   cout << "Pleae choose...\n";
   cout << "[1] - calculate a new operation\n";
   cout << "[2] - exit the program...\n";
   cin >> variables[3];
   if (variables[3] == 1) {
       system("cls");
       main();
   }
   else if (variables[3] == 2)
       return 0;
   else
       return 0;
   system("cls");
   cout << "Ending calculator...\n";
   variables[4] = getch();
   return 0;
}

Is each Case like doing if (choice ==1)? Like its giving a section for possible user input? And when you have "int variables[5]" The 5 stands for up to 5 different variables?

Just wanted to know, because if Im right I could probably make a program a lot easier. Thanks.

[Mephisto]

Not too sure what you mean...But!  The array has 5 elements in it, index 0-4, and in the switch statement the zeroth (0) element's value is put in the expression -- it evaluates to whatever it is (given in the user input) then tested in the cases.  The cases are constant values -- if variables[0] is equal to any of those cases it executes that case whos constant value matches variables[0].