0

why this recursion code fails? it fails, because it doesn't calculate factorial correctly (this is just the recursive function, you know), i.e. if I run "factorial(3)", it yields "2" instead of "6" (factorial (3) is 6, not 2). So it has a bug, doesn't it?

int factorial(int n){
    if(n>1) return n*factorial(--n);
    else return 1;    
}

if I use n-1 instead of --n, it fixes the bug? -->Moreover, if I use n--, I create a infinite loop I don't really get it

froycard
  • 227
  • 2
  • 3
  • 10
  • How does it fail with n-1? – ale10ander Mar 31 '18 at 16:13
  • Define "fails". What error message do you get with `--n`? And you can't get an infinite loop, you'll get stack overflow with `n--` because you're passing the value of `n` and decrementing only after the recursive call returns, which it won't because it'll never hit the base case. – pjs Mar 31 '18 at 16:14
  • Sounds like your compiler is evaluating the prefix increment prior to the multiplication. Since there's no reason to mutate `n`, just stick with `n-1` as the argument to the recursive call. – pjs Mar 31 '18 at 16:29

1 Answers1

2

Modifying (--n) and elsewhere using the value of a variable (n*) in the same expression, is Undefined Behavior.

pjs
  • 16,350
  • 4
  • 23
  • 44
Cheers and hth. - Alf
  • 135,616
  • 15
  • 192
  • 304
  • I didn't know the statement: " n*factorial(--n) was an " Undefined Behavior ", I think I need to look it up more closely on this matter. – froycard Mar 31 '18 at 17:04
  • 1
    @froycard Look at the last item in the "Examples in C and C++" section of the linked Wikipedia article. – pjs Mar 31 '18 at 17:08
  • Well, I definitely was not aware of this "Indefinite Behavior" since I was coding in C ++. I wonder why, anyway, can it not be solved by the compiler by applying the order of evaluation or by precedence? Or evaluating the expression from left to right when the compiler reads any line of code? I dare say that it could also be labeled as "an imperfection of the C / C ++ language". – froycard Mar 31 '18 at 22:07
  • @froycard: The old rationale of [unspecified order of evaluation of sub-expressions](http://en.cppreference.com/w/cpp/language/eval_order) was that it gave more optimization possibilities. As technology has progressed that's not necessarily so any more: processors reorder things and employ speculative evaluations, and languages like Java seem to have proved that predictable well-defined expression order can be just as efficient. And [with C++17 we got some more ordering](https://stackoverflow.com/questions/38501587/what-are-the-evaluation-order-guarantees-introduced-by-c17/38501596) – Cheers and hth. - Alf Mar 31 '18 at 22:56
  • Thanks for all the information and comments about this topic. I think I have learnt something new today, and this tip (Undefined Behavior) is something to get reckoned with during my program projects. Thank you all folks! – froycard Apr 01 '18 at 02:05