Valhalla Legends Forums Archive | C/C++ Programming | Problem with my code

AuthorMessageTime
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.
April 22, 2004, 11:22 PM
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.
April 22, 2004, 11:45 PM
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]
// 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;
}
[/code]
April 23, 2004, 1:31 AM
Eli_1
[quote author=j0k3r link=board=30;threadid=6430;start=0#msg56382 date=1082682328]
if (variables[3] == 1) {
system("cls");
main(); // <----
}[/quote]
Eww.
April 23, 2004, 2:15 AM
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
April 23, 2004, 2:23 AM
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:
[quote] else if (variables[3] == 2)
return 0;
else
return 0;
system("cls");
cout << "Ending calculator...\n";
variables[4] = getch();
return 0;[/quote]

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.
April 23, 2004, 2:35 AM
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?
April 23, 2004, 2:40 AM
Eli_1
[quote author=Mephisto link=board=30;threadid=6430;start=0#msg56402 date=1082688026]
Also, I'm making it Windows specific with Sleep() aswell.[/quote]What about iostream.h?

[quote author=Mephisto link=board=30;threadid=6430;start=0#msg56402 date=1082688026]
[code]
cout << "Bad input option...restarting...\n";
Sleep(2000); // so they have time to read the error message[/code]
[/quote]Why not use getchr();?

[quote]
Linux uses clear[/quote]
Does Linux have conio.h? And if it does, isn't there a clrscr();?
April 23, 2004, 2:51 AM
iago
[quote author=Eli_1 link=board=30;threadid=6430;start=0#msg56405 date=1082688687]
[quote author=Mephisto link=board=30;threadid=6430;start=0#msg56402 date=1082688026]
Also, I'm making it Windows specific with Sleep() aswell.[/quote]What about iostream.h?

[quote author=Mephisto link=board=30;threadid=6430;start=0#msg56402 date=1082688026]
[code]
cout << "Bad input option...restarting...\n";
Sleep(2000); // so they have time to read the error message[/code]
[/quote]Why not use getchr();?

[quote]
Linux uses clear[/quote]
Does Linux have conio.h? And if it does, isn't there a clrscr();?

[/quote]

Linux has iostream.

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

April 23, 2004, 2:53 AM
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. :P
April 23, 2004, 2:54 AM
Myndfyr
[quote author=Mephisto link=board=30;threadid=6430;start=0#msg56396 date=1082687006]
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
[/quote]

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]
#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);
}
[/code]

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.
April 23, 2004, 4:28 AM
Yoni
[quote author=Eli_1 link=board=30;threadid=6430;start=15#msg56408 date=1082688858]
Oh, I didn't think Linux had iostream. That was why I switched to stdio.h, which I'm much happier with anyway. :P
[/quote]
iostream is standard C++. Any good implementation of C++ on any platform has it.
April 23, 2004, 12:45 PM
iago
[quote author=Mephisto link=board=30;threadid=6430;start=0#msg56384 date=1082683863]
[code]
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;
}
[/code]
[/quote]

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.
April 23, 2004, 1:39 PM
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.
April 24, 2004, 1:26 AM
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].
April 24, 2004, 1:30 AM
K
I think it's generally accepted that you use arrays of a type when they're assosciated values, not just when you have more than one variable of a certain type. That's why you can name variables.

Here's how I'd do it. If you wanted to make it easier to add or modify, I'd use an array of char's for each operator and look it up by index, as well as using a function pointer to choose the correct operation, but that would be a little more hardcore than you need.

[code]
#include <iostream>
#include <cmath>
// this is a small program and there are no
// naming conflicts. using the whole namespace
// would be ok.

using std::fmod;
using std::cout;
using std::cin;
using std::endl;

void show_commands();
void get_input(float& a, float& b);

enum Operation
{
   OP_Addition = 1, OP_Subtraction,
   OP_Multiplication, OP_Division,
   OP_Modulus, OP_Quit
};

int main()
{
   int nChoice;   // the user's choice of operations.
   float fVar1, fVar2; // the two operands

   cout << "Simple calculator." << endl;
   show_commands();

   do
   {
      cout << "Choice: ";
      cin >> nChoice;

      if ((nChoice >= OP_Addition) && (nChoice < OP_Quit))
         get_input(fVar1, fVar2);

      switch(nChoice)
      {
      case OP_Addition:
         cout << fVar1 << '+' << fVar2 << '=' << (fVar1 + fVar2) << endl;
         break;

      case OP_Subtraction:
         cout << fVar1 << '-' << fVar2 << '=' << (fVar1 - fVar2) << endl;
         break;

      case OP_Multiplication:
         cout << fVar1 << '*' << fVar2 << '=' << (fVar1 * fVar2) << endl;
         break;

      case OP_Division:
         if (fVar2 != 0.0f)
            cout << fVar1 << '/' << fVar2 << '=' << (fVar1 / fVar2) << endl;
         else
            cout << "Divide by zero." << endl;

         break;

      case OP_Modulus:
         cout << fVar1 << '%' << fVar2 << '=' << fmod(fVar1, fVar2) << endl;
         break;

      case OP_Quit:
         break;

      default:
         cout << "Bad choice!" << endl;
         show_commands();
         break;
      }

   }while(nChoice != OP_Quit);

   return 0;
}



void show_commands()
{
   cout << "[1] Addition" << endl
       << "[2] Subtraction" << endl
       << "[3] Multiplication" << endl
       << "[4] Division" << endl
       << "[5] Modulus" << endl
       << "[6] Exit Program" << endl;
}

void get_input(float& a, float& b)
{
   cout << "First operand: ";
   cin >> a;
   cout << "Second operand: ";
   cin >> b;
}
[/code]
April 24, 2004, 3:29 AM
Mephisto
What I would have done if I was going to make a calculator I would later expand would be to create a calculator class and add member functions for each operation taking place. Maybe even use template functions to pass into the function either floats or integers. This would be like K said above a bit too hardcore for what you need, but it's just an idea after you get these concepts down.

Again, apologies for using recursion in main(), I had no idea that memory consumption applied to recursing main(), it just didn't occur to me...And as iago puts it, "just because you CAN do it, doesn't mean you SHOULD!"
April 24, 2004, 6:08 PM

Search