Skip to main content
Code Review

Return to Answer

replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link

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 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.

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.

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.

clarified comment on string concatenation
Source Link
Edward
  • 67.2k
  • 4
  • 120
  • 284

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.

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;

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.

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.

Source Link
Edward
  • 67.2k
  • 4
  • 120
  • 284

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;

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.

lang-cpp

AltStyle によって変換されたページ (->オリジナル) /