Putting using namespace std
at the top of every program is a bad habit a bad habit that you'd do well to avoid.
Putting using namespace std
at the top of every program is a bad habit that you'd do well to avoid.
Putting using namespace std
at the top of every program is a bad habit that you'd do well to avoid.
I see a number of things that may help you improve your code.
Fix your formatting
The code as posted has inconsistent indenting which makes it hard to read. Choose a particular style and apply it consistently to make your code easier to read.
Don't abuse using namespace std
Putting using namespace std
at the top of every program is a bad habit that you'd do well to avoid.
Validate the input
The code does not seem to be able to handle negative numbers as in the expression:
-3+2
If it's not intended to handle negative numbers, it would be good to let the user know. If it is, then there's a bug that should be fixed.
Don't use std::endl
if you don't really need it
The difference betweeen std::endl
and '\n'
is that '\n'
just emits a newline character, while std::endl
actually flushes the stream. This can be time-consuming in a program with a lot of I/O and is rarely actually needed. It's best to only use std::endl
when you have some good reason to flush the stream and it's not very often needed for simple programs such as this one. Avoiding the habit of using std::endl
when '\n'
will do will pay dividends in the future as you write more complex programs with more I/O and where performance needs to be maximized.
Don't use system("PAUSE")
There are two reasons not to use system("cls")
or system("PAUSE")
. The first is that it is not portable to other operating systems which you may or may not care about now. The second is that it's a security hole, which you absolutely must care about. Specifically, if some program is defined and named PAUSE
or pause
, your program will execute that program instead of what you intend, and that other program could be anything. First, isolate these into a seperate functions pause()
and then modify your code to call those functions instead of system
. Then rewrite the contents of those functions to do what you want using C++. For example:
void pause() {
getchar();
}
General portability
This code could be made portable if you omit the Windows-only include files #include "stdafx.h"
. If you must have stdafx.h
, consider wrapping it so that the code is portable:
#ifdef WINDOWS
#include "stdafx.h"
#endif
Separate input, output and calculation
To the degree practical it's usually good practice to separate input, output and calculation for programs like this. By putting them in separate functions, it isolates the particular I/O for your platform (which is likely to be unique to that platform or operating system) from the logic of the program (which does not depend on the underlying OS). So for example, the main
program might call a function to fetch and validate the input, a second function to actually perform the mathematical operations and then a third to emit the results.
Omit spurious parentheses
The argument to a case
statement or return
statement does not need parentheses, so instead of this:
case('#'):
One could write this:
case '#':
Consolidate strings
Right now, the string "Input not recognized"
appears four times in the program. I'd recommend creating a const
variable instead and then using that instead of repeating the string.