Printing a line changes the return value of my function

Question

I've just begun learning C++ today and have some experience with other languages (Ruby). I'm understanding most of the basics, but I can't for the life of me figure out why this function (which returns the sum of the digits of an integer) only works correctly when I print the sum before returning the sum.

The code in question returns 32766 instead of 8:

#include <iostream>
#include <cmath>
using namespace std;
#include <string>

int sumOfDigits(int x) {
    int i = 1;
    int inc = 1;
    while(i < x){
        i *= 10;
        inc += 1;
    }
    int size = inc;
    int digs[inc];
    inc = 0;
    int z = x;
    i /= 10;
    while(z > 0) {
        digs[inc] = (z/i);
        z -= i * digs[inc];
        i /= 10;
        inc += 1;
    }
    int sum = 0;
    for (inc = 0; inc < size; inc += 1) {
        sum += digs[inc];
    }
    return sum;
}

int main(){
    cout << sumOfDigits(125); // prints 32766
    return 0;
}

but this piece of code that prints "hello world" before the return statement outputs 8 as expected.

#include <iostream>
#include <cmath>
using namespace std;
#include <string>


int sumOfDigits(int x) {
    int i = 1;
    int inc = 1;
    while(i < x){
        i *= 10;
        inc += 1;
    }
    int size = inc;
    int digs[inc];
    inc = 0;
    int z = x;
    i /= 10;
    while(z > 0) {
        digs[inc] = (z/i);
        z -= i * digs[inc];
        i /= 10;
        inc += 1;
    }
    int sum = 0;
    for (inc = 0; inc < size; inc += 1) {
        sum += digs[inc];
    }
    cout << "hello world";
    return sum;
}

int main(){
    cout << sumOfDigits(125);
    return 0;
}

`int digs[inc];` is not standard C++. See [Why aren't variable-length arrays part of the C++ standard?](https://stackoverflow.com/questions/1887097/). Use `std::vector` instead, or simply rewrite the code to not require an array at all. — Remy Lebeau, Aug 17 '22 at 04:07
1. variable length arrays like `digs` are not C++ standard, and shouldn't be relied on. 2. The calculation of `size` based on the *miscalculation* of `inc` is wrong. `inc` should start at 0, not 1. Frankly, all of this is pointless if all you want to do is capture each digit to an accumulator. No array should be needed nor used. — WhozCraig, Aug 17 '22 at 04:09

score 1 · Answer 1 · answered Aug 17 '22 at 04:11

1

You read digs in places you did not write. You can easily verify this by printing the accesses:

while(z > 0) {
    std::cout <<"Writing digs[" <<inc <<"] = "<<(z/i) <<"\n";
    //...
}
//...
for (inc = 0; inc < size; inc += 1) {
    std::cout <<"Reading digs[" <<inc <<"] => " <<digs[inc] <<"\n";
    //...
}

Result:

Writing digs[0] = 1
Writing digs[1] = 2
Writing digs[2] = 5
Reading digs[0] => 1
Reading digs[1] => 2
Reading digs[2] => 5
Reading digs[3] => 32125

Note that last read is reading an element that was never initialized, and thus can contain anything. Formally, reading uninitialized memory is undefined behavior.

In practice, this will result in reading garbage that depends on what happens to be there.

answered Aug 17 '22 at 04:11

spectras

13,105
2
31
53

thanks a lot, that makes sense. I thought i'd get an error when i read an index that doesn't exist, so i'll definitely consider that stuff in the future. – vulcan_ocelot Aug 17 '22 at 04:26
@vulcan_ocelot> note that in this case the index "exists" in the sense that `digs` has 4 elements (indexed 0 to 3). The issue is those elements are not initialized, so they contain garbage. Then the program writes elements 0 to 2, but not element 3, which still contains garbage when it is read in the last loop. – spectras Aug 17 '22 at 04:31
Had you initialized the array (eg `int digs[size] = {};`, that unintuitive syntax will zero the entire array), then the last read would read `0`. But arguably this would be worse: at least with reading garbage you noticed there was a problem and you will be able to fix the loop. – spectras Aug 17 '22 at 04:35
*"In practice, this will result in reading garbage that depends on what happens to be there."* I'd phrase that differently. *In practice* invoking UB like this can do all kinds of funny things. It may usually/often do what you write, but even *in practice* it may do something else entirely (if not today, then after you upgrade the compiler). – hyde Aug 17 '22 at 05:30

score 0 · Answer 2 · edited Aug 17 '22 at 04:10

#include<iostream>

using namespace std;

void sum_of_digits(int);

int main(){

  int num;

  cin>>num;

   sum_of_digits(num);
}

void sum_of_digits(int x){
   static int num=x;

   int sum=0;

   while(x>0){
      int last_digit=x%10;

      sum+=last_digit;

      x/=10;
   }

   cout<<"sum of digits of "<<num<<" is "<<sum<<endl;
}

in your program u have made multiple assigments,re-assignment, i think problem lies there. purpose of first while loop is also not clear..

Printing a line changes the return value of my function

2 Answers2