Summation and multiplication of digits of a number

Question

This program takes digits of a number and calculate summation and multiplication of its digits .How can I make this code more OOP ?

//Calculate Sum and multiplication of digits of a number .

#include "stdafx.h"
#include <iostream>
#include <vector>

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

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

    cout << "Calculate Sum and multiplication of digits of a number ." << "\n" << "\n";
    cout << "------------------------------------" << "\n";
    cout << "Enter a whole number : ";

    cin >> number;

    if (number > 0)         //Condition for inputed number
    {

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


        for (int i = 0; i < digits.size(); i++) // summation
        {
            sum += digits[i];
        }
        cout << "sum of digits = " << sum << "\n";

        for (int j = 0; j < digits.size(); j++) //multiplication
        {
            multp *= digits[j];
        }

        cout << "Multiplication of digits = " << multp << "\n";
    }


    else
    {
        cout << "Pleas enter a positive number ." << "\n";
    }


    cout << "\n" << "\n" << "------------------------------------" << "\n";
    system("pause");
    return 0;
}

Thanks in advance.

Answer 1

First, i would question the need for a collection of the digits. All you really need is the summation and multiplications results:

while ( number )
{
    int temp = number % 10;
    sum += temp;
    multp *= temp;
    number /= 10;
}

Putting this into a class is relatively simple

#include <iostream>

using std::cout;
using std::cin;
class CalcDigits
{
    int number = 0;
    int sum = 0;
    int multp = 1;
    void CalculateDigits()
    {
        while ( number )
        {
            int temp = number % 10;
            sum += temp;
            multp *= temp;
            number /= 10;
        }
    }
public:
    CalcDigits()
    {

    }
    CalcDigits( int num )
    {
        number = num;
        CalculateDigits();
    }
    void SetNumber( int num )
    {
        number = num;
        CalculateDigits();
    }
    int GetNumber()
    {
        return number;
    }
    int GetSum()
    {
        return sum;
    }
    int GetMultp()
    {
        return multp;
    }
};
int main()
{
    int number;

    cout << "Calculate Sum and multiplication of digits of a number ." << "\n" << "\n";
    cout << "------------------------------------" << "\n";
    cout << "Enter a whole number : ";

    cin >> number;

    if ( number > 0 )         //Condition for inputed number
    {
        CalcDigits cd( number );

        cout << "sum of digits = " << cd.GetSum() << "\n";
        cout << "Multiplication of digits = " << cd.GetMultp() << "\n";
    }
    else
    {
        cout << "Pleas enter a positive number." << "\n";
    }


    cout << "\n" << "\n" << "Press enter to continue" << "\n";
    cin.ignore();
    cin.get();
}

Notice how main is reduced to just the interaction with the user and the class handles all the calculation.

Also using System("Pause"), while convenient, is non-standard and very platform specific, and the alternative isn't very onerous.

Answer 2

Using std::accumulatewould be more idiomatic. Also use auto whenever possible.

#include <algorithm>
#include <functional> // requires C++14, else use a lambda for the predicate
auto sum = std::accumulate( digits.cbegin(), digits.cend(), 0 );
auto prod = std::accumulate( digits.cbegin(), digits.cend(), 1, std::multiplies<>);

---

Comment on the accepted answer, which I at first mistook for an edit of the question:

• Class names should be nouns, function names should be verbs. As in DigitsCalculator.

• Your no argument constructor can be written as CalcDigits = default.

• Having a setter and getter for a private variable effective eliminates the encapsulation. There are divergent opinions on this topic. Some say a setter is better, because you can add logic for accessing the variable later. I, personally, have never "added logic" to a setter, and would just make the variable public.

• You have an invariant: CalcDigits c; c.SetNumber(42); c.GetSum(); c.GetMul();. Interfaces should be "easy to use correctly, difficult to use incorrectly.

• Overall, I like the new look better. You have separation of concerns (command line communication and actual calculation). You also have an encapsulated object, that you can pass around with clear intent.

Answer 3

So the obvious hack for digits is to actually read the number into a string and then walk over the characters of that string.

You can write your own lambda functions that do the correct thing for a given char and use std::accumulate with them.