6
\$\begingroup\$

I have been learning C++ programming. What you see here is all that I know how to do at this point. I look forward to learning more and I know there are better ways to accomplish a task such as calculating, or a less verbose method for displaying information.

I want to hear from more experienced developers and engineers what they would have done better for the following code.

As I am learning new techniques I will be more inclined to pick up the better, more efficient methods of accomplishing a task, however simple it may be. I have heard that it is common for beginners to harbor bad habits. It would be the greater good as you may have to work with the likes of me in the future.

#include <iostream>

using namespace std;

int main()
{
int a;
int b;
int sum;
int diff;
int prod;
int quot;
int remain;

cout << "Welcome to the Basic Calculator for integers!" << endl;
cout << "Enter two numbers to see the difference, and please: one at a time!" << endl;

cout << "First Number: ";
cout.flush();
cin >> a;

cout << "Second Number: ";
cout.flush();
cin >> b;
cout << "\n";

sum = a + b;
diff = a - b;
prod = a * b;

quot = a/b;
remain = a%b;

cout << "Basic calculations for your two numbers: \n" << "Sum: " << sum << "\n" << "Difference: " << diff << "\n" << "Product: " << prod << "\n" << "Quotient: " << quot << " with a remainder of " << remain << endl;
}

I know that my white space looks terrible and indenting is a good thing, but for ease of formatting it looks a little uglier than I would like.

\$\endgroup\$
10
\$\begingroup\$

I see a number of things that could help you improve your code.

Decompose your program into functions

All of the logic here is in main in one chunk of code. It may be better to decompose this into separate functions, such as separate ones for input, calculation and output.

Declare variables as late as possible

Rather than using the old C-style of declaring all variables at the top of a function, use the modern C++-style and declare variables as late as possible. Doing so can sometimes help the compiler figure out register allocation, resulting in faster, smaller code.

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. It is particularly bad to put it into a header file, so please don't do that.

Use constant string concatenation

This code currently starts with these two lines:

cout << "Welcome to the Basic Calculator for integers!" << endl;
cout << "Enter two numbers to see the difference, and please: one at a time!" << endl;

The two observations I would make are that the first one doesn't really need endl because just a new line will do. (The difference is that std::endl additionally flushes the buffer.) The second observation is that you don't really need to call << four times. Instead, you can use string concatenation and call it only twice:

cout << "Welcome to the Basic Calculator for integers!\n"
    "Enter two numbers to see the difference, and please: one at a time!" << endl;

To be clear, when two static strings like that are right next to each other, the compiler is obligated (by the standard) to treat them as a single string.

Sanitize user input better

The code has a potential problem as posted. If I enter a string such as "Edward" to instead of a number, the program stays in an endless loop. It would be better to read a (text) line in and then convert it to a number. Users can do funny things and you want your program to be robust.

Pay attention to mathematical problems

If the user chooses zero for the second number, the program is likely to crash because of a divide-by-zero. Your code should check the input for zero.

\$\endgroup\$
  • \$\begingroup\$ Thank you for such a complete review! I am going to practice string concatenation and declaring variables close to where they are being used. On the topic of sanitizing user input: Would you be able to provide an example of how the program could better receive user input? I am so new to this that I don't quite know how to catch exceptions. \$\endgroup\$ – brndng Oct 2 '15 at 20:08
  • 1
    \$\begingroup\$ See codereview.stackexchange.com/questions/105088/… for some ideas on how to sanitize user input. \$\endgroup\$ – Edward Oct 2 '15 at 22:14

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.