3

I was trying to calculate a factorial using recursion like this:

#include <iostream>

using namespace std;

int factorial(int a)
{
    if(a == 0)
    {
        return 1;
    }
    return a*factorial(--a);
}

int main()
{
    int a;
    cin >> a;

    cout << factorial(a) << endl;

    return 0;
} 

and it wasn't working. Then, I made a small change:

#include <iostream>

using namespace std;

int factorial(int a)
{
    if(a == 0)
    {
        return 1;
    }
    return a*factorial(a-1);
}

int main()
{
    int a;
    cin >> a;

    cout << factorial(a) << endl;

    return 0;
} 

... and it started working!

The problem is that I don't see any difference between these codes: Why didn't it work in the first code?

Adrian Mole
  • 49,934
  • 160
  • 51
  • 83
DGX37
  • 304
  • 3
  • 12
  • @eeorika Although I am not disputing your duplicate-closure, I think my answer can still be held as helpful, as the target you specified doesn't really address the specific problem here, which is use of `--a` *versus* `a-1`. – Adrian Mole Jul 27 '20 at 15:56

1 Answers1

10

In your first code sample, the following line has undefined behaviour:

return a * factorial(--a);

This is because there is nothing in the C++ Standard that dictates whether or not the 'old' or 'new' (decremented) value of a is used to multiply the return value of the factorial function.

Compiling with clang-cl gives the following:

warning : unsequenced modification and access to 'a' [-Wunsequenced]

In your second code sample, there is no such ambiguity, as a is not modified.

Adrian Mole
  • 49,934
  • 160
  • 51
  • 83