Skip to main content
added 75 characters in body
Source Link
user128454
user128454

Good job on your first program!

Before moving on to the review, a quick note on using namespace std

As long as you do it in a cpp file (and not a header), bringing in symbols from a namespace one at a time is absolutely fine, since you are in control:

#include <iostream>

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

Errors

main's return value

The main()'s return value is for program status, not program output. I suspect you meant to do:

cout << "the sum is: " << addTwoNumbers() << endl;
return 0;

Good practice improvements

Avoid globals

int x;
int sum = 0;
int i = 0;

These have no business being global variables, they belong inside of your functions, as close to their first use as possible:

for(int i=0; i<2;)
{
    int x;
    cin >> x;

istream::fail() vs operator bool()

instead of if(std::cin.fail()) you should simply do if(std::cin), it actually covers more scenarios as wellEdit Disregard this recommendation. See the comment below.

instead of if(std::cin.fail()) you should simply do if(std::cin), it actually covers more scenarios as well.

istream::ignore()

Whenever you use an arbitrary number like 256, it's a sign that something is not how it should be. In this specific case, you want to do cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n')

From a higher level:

addTwoNumbers() does too many things

Try to keep your functions short, single-purpose and reusable.

As guidance, look at this version of your main().

int main() {
  cout << "Enter two integers" << endl;

  int first = getNumber();
  int second = getNumber();

  cout << "the sum is: " << (first + second) << endl;
  
  return 0;
}

getNumber() would now be be a usefull and reusable piece of code, which is what functions should be.

Good job on your first program!

Before moving on to the review, a quick note on using namespace std

As long as you do it in a cpp file (and not a header), bringing in symbols from a namespace one at a time is absolutely fine, since you are in control:

#include <iostream>

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

Errors

main's return value

The main()'s return value is for program status, not program output. I suspect you meant to do:

cout << "the sum is: " << addTwoNumbers() << endl;
return 0;

Good practice improvements

Avoid globals

int x;
int sum = 0;
int i = 0;

These have no business being global variables, they belong inside of your functions, as close to their first use as possible:

for(int i=0; i<2;)
{
    int x;
    cin >> x;

istream::fail() vs operator bool()

instead of if(std::cin.fail()) you should simply do if(std::cin), it actually covers more scenarios as well.

istream::ignore()

Whenever you use an arbitrary number like 256, it's a sign that something is not how it should be. In this specific case, you want to do cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n')

From a higher level:

addTwoNumbers() does too many things

Try to keep your functions short, single-purpose and reusable.

As guidance, look at this version of your main().

int main() {
  cout << "Enter two integers" << endl;

  int first = getNumber();
  int second = getNumber();

  cout << "the sum is: " << (first + second) << endl;
  
  return 0;
}

getNumber() would now be be a usefull and reusable piece of code, which is what functions should be.

Good job on your first program!

Before moving on to the review, a quick note on using namespace std

As long as you do it in a cpp file (and not a header), bringing in symbols from a namespace one at a time is absolutely fine, since you are in control:

#include <iostream>

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

Errors

main's return value

The main()'s return value is for program status, not program output. I suspect you meant to do:

cout << "the sum is: " << addTwoNumbers() << endl;
return 0;

Good practice improvements

Avoid globals

int x;
int sum = 0;
int i = 0;

These have no business being global variables, they belong inside of your functions, as close to their first use as possible:

for(int i=0; i<2;)
{
    int x;
    cin >> x;

istream::fail() vs operator bool()

Edit Disregard this recommendation. See the comment below.

instead of if(std::cin.fail()) you should simply do if(std::cin), it actually covers more scenarios as well.

istream::ignore()

Whenever you use an arbitrary number like 256, it's a sign that something is not how it should be. In this specific case, you want to do cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n')

From a higher level:

addTwoNumbers() does too many things

Try to keep your functions short, single-purpose and reusable.

As guidance, look at this version of your main().

int main() {
  cout << "Enter two integers" << endl;

  int first = getNumber();
  int second = getNumber();

  cout << "the sum is: " << (first + second) << endl;
  
  return 0;
}

getNumber() would now be be a usefull and reusable piece of code, which is what functions should be.

deleted 8 characters in body
Source Link
user128454
user128454

Good job on your first program!

Before moving on to the review, a quick note on using namespace std

As long as you do it in a cpp file (and not a header), bringing in symbols from a namespace one at a time is absolutely fine, since you are in control:

#include <iostream>

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

Errors

main's return value

The main()'s return value is for program status, not program output. I suspect you meant to do:

cout << "the sum is: " << addTwoNumbers() << endl;
return 0;

Good practice improvements

Avoid globals

int x;
int sum = 0;
int i = 0;

These have no business being global variables, they belong inside of your functions, as close to their first use as possible:

for(int i=0; i<2;)
{
    int x;
    cin >> x;

istream::fail() vs operator bool()

instead of if(std::cin.fail()) you should simply do if(std::cin), it actually covers more scenarios as well.

istream::ignore()

Whenever you use an arbitrary number like 256, it's a sign that something is not how it should be. In this specific case, you want to do cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n')

From a higher level:

addTwoNumbers() does too many things

Try to keep your functions short, single-purpose and reusable.

As guidance, look at this version of your main().

int main() {
  cout << "Enter two integers" << endl;

  int first = getNumber();
  int second = getNumber();

  cout << "the sum is: " << (first + second) << endl;
  
  return 0;
}

getNumber() would now assumed tobe be a usefull and reusable piece of code, which is what functions should be.

Good job on your first program!

Before moving on to the review, a quick note on using namespace std

As long as you do it in a cpp file (and not a header), bringing in symbols from a namespace one at a time is absolutely fine, since you are in control:

#include <iostream>

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

Errors

main's return value

The main()'s return value is for program status, not program output. I suspect you meant to do:

cout << "the sum is: " << addTwoNumbers() << endl;
return 0;

Good practice improvements

Avoid globals

int x;
int sum = 0;
int i = 0;

These have no business being global variables, they belong inside of your functions, as close to their first use as possible:

for(int i=0; i<2;)
{
    int x;
    cin >> x;

istream::fail() vs operator bool()

instead of if(std::cin.fail()) you should simply do if(std::cin), it actually covers more scenarios as well.

istream::ignore()

Whenever you use an arbitrary number like 256, it's a sign that something is not how it should be. In this specific case, you want to do cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n')

From a higher level:

addTwoNumbers() does too many things

Try to keep your functions short, single-purpose and reusable.

As guidance, look at this version of your main().

int main() {
  cout << "Enter two integers" << endl;

  int first = getNumber();
  int second = getNumber();

  cout << "the sum is: " << (first + second) << endl;
  
  return 0;
}

getNumber() would now assumed to be a usefull and reusable piece of code, which is what functions should be.

Good job on your first program!

Before moving on to the review, a quick note on using namespace std

As long as you do it in a cpp file (and not a header), bringing in symbols from a namespace one at a time is absolutely fine, since you are in control:

#include <iostream>

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

Errors

main's return value

The main()'s return value is for program status, not program output. I suspect you meant to do:

cout << "the sum is: " << addTwoNumbers() << endl;
return 0;

Good practice improvements

Avoid globals

int x;
int sum = 0;
int i = 0;

These have no business being global variables, they belong inside of your functions, as close to their first use as possible:

for(int i=0; i<2;)
{
    int x;
    cin >> x;

istream::fail() vs operator bool()

instead of if(std::cin.fail()) you should simply do if(std::cin), it actually covers more scenarios as well.

istream::ignore()

Whenever you use an arbitrary number like 256, it's a sign that something is not how it should be. In this specific case, you want to do cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n')

From a higher level:

addTwoNumbers() does too many things

Try to keep your functions short, single-purpose and reusable.

As guidance, look at this version of your main().

int main() {
  cout << "Enter two integers" << endl;

  int first = getNumber();
  int second = getNumber();

  cout << "the sum is: " << (first + second) << endl;
  
  return 0;
}

getNumber() would now be be a usefull and reusable piece of code, which is what functions should be.

Source Link
user128454
user128454

Good job on your first program!

Before moving on to the review, a quick note on using namespace std

As long as you do it in a cpp file (and not a header), bringing in symbols from a namespace one at a time is absolutely fine, since you are in control:

#include <iostream>

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

Errors

main's return value

The main()'s return value is for program status, not program output. I suspect you meant to do:

cout << "the sum is: " << addTwoNumbers() << endl;
return 0;

Good practice improvements

Avoid globals

int x;
int sum = 0;
int i = 0;

These have no business being global variables, they belong inside of your functions, as close to their first use as possible:

for(int i=0; i<2;)
{
    int x;
    cin >> x;

istream::fail() vs operator bool()

instead of if(std::cin.fail()) you should simply do if(std::cin), it actually covers more scenarios as well.

istream::ignore()

Whenever you use an arbitrary number like 256, it's a sign that something is not how it should be. In this specific case, you want to do cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n')

From a higher level:

addTwoNumbers() does too many things

Try to keep your functions short, single-purpose and reusable.

As guidance, look at this version of your main().

int main() {
  cout << "Enter two integers" << endl;

  int first = getNumber();
  int second = getNumber();

  cout << "the sum is: " << (first + second) << endl;
  
  return 0;
}

getNumber() would now assumed to be a usefull and reusable piece of code, which is what functions should be.