2
\$\begingroup\$

This is a simple C++ program I had to write for class. It prompts the user to input an integer and then outputs both the individual digits of the number and the sum of the digits. How do I improve this?

For example:

Input: 3456
Output: 
3 4 5 6
Sum of all digits: 19

Input: 1234
Output:
1 2 3 4
Sum of all digits: 10
#include <iostream>
#include <iomanip>

using namespace std;
int main()
{
    int base;
    int first;
    int second;
    int third;
    int fourth;
    int sum;

    cout << " Please enter a whole number." << endl;
    cin >> base ;

    first = base / 1000;
    second = base / 100 % 10;
    third =  base / 10 % 10;
    fourth = base % 10;

    sum = first + second + third + fourth;

    cout << first << " " << second << " " << third<<  " " << fourth <<  " " << endl;
    cout << "Sum of the digits: " << sum << endl;

    return 0;
}
\$\endgroup\$
4
\$\begingroup\$
  1. Please stop using using namespace std. Read this
  2. Consider using a std::vector to hold the digits of a number. Here it seems as though you can assume the user is inputting a number with four digits. But that might not always be the case. A vector is dynamic.
  3. This is...uh, well:

    first = base / 1000;
    second = base / 100 % 10;
    third =  base / 10 % 10;
    fourth = base % 10;
    

    Consider using a while loop to get all digits of the number.

    while(number)
    {
        digits.push_back(number%10);
        number /= 10;
    }
    

    Note the digits will be in reverse. You can use std::reverse() to reverse the digits such that the first element in the vector is the first digit of the number.

  4. Use std::accumulate() to obtain the sum of all digits. You don't have to do this in the while loop.

Here is everything together. Ideally, you would want to put the while loop and summing operation in another function.

#include <iostream>
#include <vector>
#include <algorithm>

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



int main()
{
    int number, sum;
    vector<int> digits;

    cout << "Enter a whole number: ";
    cin >> number;

    while(number)
    {
        digits.push_back(number%10);
        number /= 10;
    }


    std::reverse(digits.begin(),digits.end());
    sum = std::accumulate(digits.begin(),digits.end(),0);

    //C++11 range-based for loop
    for(auto i : digits)
        cout << i << " ";

    cout << "\nSum of the digits: ";
    cout << sum << endl;

}
| improve this answer | |
\$\endgroup\$
4
\$\begingroup\$

An immediate question is what happens if the base number has more than 4 digits?

Another question is do you really need to convert the number entered into an int? Hint: you only need individual digits, and each character represents one.

Solution:

  • Initialize sum to 0
  • Read input character by character
    • (try to) Convert the character to a number
    • Add the result to sum
| improve this answer | |
\$\endgroup\$
3
\$\begingroup\$

As others have suggested, you should use a loop to extract the digits of the number using % 10 in each iteration step. While iterating, you could also add up the numbers to calculate the sum, so you don't have to iterate them again in a second pass.

Since the % 10 operation will extract the digits in reverse order but you want to use the original order for printing, you could accumulate the digits by prepending in a list. That way the resulting list will have the original order preserved.

#include <iostream>
#include <list>

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

int main()
{
    int number;

    cout << "Enter a whole number: ";
    cin >> number;

    int sum = 0;
    list<int> digits;
    while (number)
    {
        int digit = number % 10;
        sum += digit;
        digits.push_front(digit);
        number /= 10;
    }

    for (auto digit : digits) cout << digit << " ";

    cout << "\nSum of the digits: ";
    cout << sum << endl;
}

Additional remarks:

  • The #include <iomanip> was pointless: you didn't use anything from this library. Make it a habit to only include what you really use.
  • Declare variables right before you use them. No need to declare everything at the top, like you did the variables first, second, and so on.
| improve this answer | |
\$\endgroup\$
2
\$\begingroup\$

One thing to consider, just because an integer is requested, doesn't mean you have to store it as an integer. If you store it as a string, not only is it in an iterable collection already, but you can easily validate the input and control what happens if there is an error, more easily:

string base;
bool good = false;
int sum = 0;
while (!good)
{
    good = true;
    cout << " Please enter a whole number." << '\n';
    cin >> base;
    for (auto c : base)
    {
        if (c >= '0' && c <= '9')
        {
            cout << c << " ";
            sum += c - '0';
        }
        else
        {
            good = false;
            break;
        }
    }
    if (good)
    {
        cout << "\nSum of the digits: " << sum << endl;
    }
    else
    {
        cout << "Only digits(0-9) please.\n";
    }
}
| improve this answer | |
\$\endgroup\$
1
\$\begingroup\$

Your program is specifically for only four numbers. Let's make it more generic. Mine doesn't include the sum, you can do it easily. Most simple programs use simple operators.

#include <iostream>
#include <cmath>
using namespace std;
int main()
{
    int count = -1;
    int copyinput = 0;
    int input = 0;
    cout<<"enter a positive whole number : ";
    cin>>input;
    copyinput = input;
    while(copyinput > 0 )
    {
        copyinput = copyinput/10;
         count = count +1;

    }
    int digit = 0;
    while(count >= 0)
    {
        digit = input/(pow(10,count));
        digit = digit % 10;
        count = count -1;
        cout<<digit<<endl;
    }   
}
| improve this answer | |
\$\endgroup\$
  • 2
    \$\begingroup\$ Welcome to Code Review! Please explain a bit more about the changes and suggestions in your code and how they work. It is relatively clear from your code but a big part of CR is the explanation of your changes. \$\endgroup\$ – SuperBiasedMan Sep 7 '15 at 14:58

Your Answer

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