Splitting an integer to its digits

Question

I want to split an integer of type unsigned long long to its digits. Any comments and suggestions are always welcome.

#include <iostream>
#include <string>

#include <vector>

using namespace std;

vector<unsigned short> IntegerDigits(unsigned long long input)
{
    vector<unsigned short> output;
    const unsigned short n = log10(ULLONG_MAX);
    unsigned long long divisor = pow(10, n);
    bool leadingZero = true;

    for (int i = 0; i < n + 1; i++)
    {
        unsigned short digit = input / divisor % 10;
        if (!leadingZero || digit != 0)
        {
            output.push_back(digit);
            leadingZero = false;
        }
        divisor /= 10;
    }

    return output;
}

void main()
{
    vector<unsigned short> output = IntegerDigits(ULLONG_MAX);

    cout << ULLONG_MAX << ": [";
    for (auto y : output)
        cout << y << ", ";
    cout << "\b\b]";

    cout << ""<<endl;

}

short n = log10(ULLONG_MAX) will this be rounded up or down? Why? — greybeard
– greybeard, Commented Oct 31, 2020 at 6:33
@greybeard: Down. Because log10(ULLONG_MAX) + 1 represents "the max number of digits" for unsigned long long. — Display Name
– Display Name, Commented Oct 31, 2020 at 6:40
@greybeard the max number of digits becomes 20. What is the problem? — Display Name
– Display Name, Commented Oct 31, 2020 at 6:51
The problem is that a rounded up pow(10, n) may not be "losslessly" assignable to an unsigned long long. — greybeard
– greybeard, Commented Oct 31, 2020 at 7:54

G. Sliepen · Accepted Answer · 2020-10-31 10:17:07Z

10

Avoid mixing floating point and integer arithmetic

As mentioned by greybeard, there is a potential problem here:

const unsigned short n = log10(ULLONG_MAX);

ULLONG_MAX is larger than can be exactly represented by a double. This means the result might not be what you expect. The same goes for pow(10, n). While you can compensate for it, it is better to find a way to calculate the length of a number without using floating point math.

Keep it simple

Unless performance is a big concern, keep it simple. You don't have to know the number of digits up front if you push trailing digits to the front of the vector, like so:

vector<unsigned short> IntegerDigits(unsigned long long input)
{
    vector<unsigned short> output;

    while (input)
    {
        output.insert(output.begin(), input % 10);
        input /= 10;
    }

    // Handle input being equal to 0
    if (output.empty())
    {
        output.push_back(0);
    }

    return output;
}

Pushing to the front of a std::vector is less efficient, but on the other hand you don't need the double<->int conversions, and you don't need to handle the leading zeros inside the loop.

Avoid using `std::endl`

Prefer using "\n" over std::endl, the latter is equivalent to the former, but also forces a flush of the output, which can be bad for performance.

Avoid backspaces in the output

You used a neat trick to get rid of the last comma without having to have extra logic inside the for-loop in main(). However, consider that the output might not just be for human consumption, but is written to a file and/or is parsed by another program. In that case, the \b characters are probably unexpected and might cause problems.

edited Oct 31, 2020 at 10:17

answered Oct 31, 2020 at 8:06

G. Sliepen

69.3k3 gold badges75 silver badges180 bronze badges

1

\$\begingroup\$ Where is push_front defined? \$\endgroup\$

Display Name
– Display Name

2020-10-31 08:30:06 +00:00
Commented Oct 31, 2020 at 8:30
\$\begingroup\$ I changed to stack instead of vector because push_front has not been defined in vector. :-) \$\endgroup\$

Display Name
– Display Name

2020-10-31 08:51:51 +00:00
Commented Oct 31, 2020 at 8:51
\$\begingroup\$ Ehr oops, I meant insert(output.begin(), ...)! \$\endgroup\$

G. Sliepen
– G. Sliepen

2020-10-31 10:16:56 +00:00
Commented Oct 31, 2020 at 10:16
1

\$\begingroup\$ Better to just append and then reverse in-place? \$\endgroup\$

Deduplicator
– Deduplicator

2020-10-31 19:43:42 +00:00
Commented Oct 31, 2020 at 19:43
\$\begingroup\$ Or use a std::deque. The are many ways one could do this, but since it's likely only a few digits will be stored, I don't think it's going to matter much. This is where I would just keep the simplest approach, and only change it when performance becomes a bottleneck and measurements have shown that this function is a big contributor. \$\endgroup\$

G. Sliepen
– G. Sliepen

2020-10-31 21:03:22 +00:00
Commented Oct 31, 2020 at 21:03

| Show 3 more comments

theProgrammer · Accepted Answer · 2020-11-01 11:48:03Z

As @G. Sliepen said, I also want to reiterate, when performance isn't a problem, ensure to make the code as simple as possible.

A modified version using stack might be written like this

void separate_digits( stack<int> &s, long long int digits )
{
    while( digits != 0 ) 
    {
        s.push( digits % 10 );
        digits /= 10;
    }
}

Displaying the digit would just require you to pop the stack, which as we know takes constant time

void print_separated_digits( stack<int> &s ) 
{
    while( !s.empty( ) )
    {
        std::cout << s.top( ) << " ";
        s.pop( );
    }
}

Stack Exchange Network

Splitting an integer to its digits

2 Answers 2

Avoid mixing floating point and integer arithmetic

Keep it simple

Avoid using `std::endl`

Avoid backspaces in the output

You must log in to answer this question.

Hot Network Questions

Splitting an integer to its digits

2 Answers 2

Avoid mixing floating point and integer arithmetic

Keep it simple

Avoid using std::endl

Avoid backspaces in the output

You must log in to answer this question.

Related

Hot Network Questions

Avoid using `std::endl`