Decrement Operator in C++

Question

I am from C background, and now I am learning OOP using C++

Below is a program that calculates factorial.

#include <iostream>

using namespace std;

void main ()
{
    char dummy;
    _int16 numb;
    cout << "Enter a number: ";
    cin >> numb;

    double facto(_int16);

    cout << "factorial = " <<facto(numb);
    cin >> dummy;
}

double facto( _int16 n )
{
    if ( n>1 )
        return ( n*facto(n-1) );
    else
        return 1;
}

The above code works fine.

But if I replace the return statement

    return ( n*facto(n-1) );

with this

    return ( n*facto(n--) );

then it doesn't work. The n-- won't decrement n by 1. Why?

I am using Visual Studio 2012

Edit:Got it! thanks :)
*also, I would like to add to the answers below: using --n will cause the n to decrement before the statement is executed. So, due to pre-decrement, the expression will become (n-1)*facto(n-1) . That is why it is better not to use pre-decrement in this case *

"The n-- won't decrement n by 1" - not true. next time verify your assumption. note: it works just as in C. — Karoly Horvath, Mar 14 '14 at 12:32
Given recent security alerts, you may want to start using {} around your if blocks. Also, `using namespace std` is bad form. — Alex Chamberlain, Mar 14 '14 at 12:33
You should not declare your prototypes inside of `main`. Also, C has pre and post increment just like C++. And what is wrong with `n - 1`? That is kind of basic for recursive calls. — crashmstr, Mar 14 '14 at 12:35
`n*facto(n--)` is undefined behavior. clang says `warning: unsequenced modification and access to 'n'` — fredoverflow, Mar 14 '14 at 12:40
@TomFenech: Google for "goto fail", for a recent example. It's easy to add a second line but forget to add braces, ending up with code exectuted unconditionally when it shouldn't be. (In this case, it caused an important security check to be bypassed.) — Mike Seymour, Mar 14 '14 at 12:49
using "--n" will not "for sure" generate "(n-1)*facto(n-1)". According to the standards, it is undefined when you introduce more than one sequence points for execution. — Abhineet, Mar 14 '14 at 13:20
@Abhineet First, thanks for your replly :) Second, i do not understand the concept of sequence points.. Well, after reading your reply to my edit, i executed my program with `(n-1)*facto(n-1)` giving n=5, and second time I executed it with `n*facto(--n)`. It calculated the factorial of 5 to be 24 in both cases because *every n* was getting decremented by 1. — ColorDeColor, Mar 14 '14 at 13:28
That's why it is called Undefined Behavior. Some times, you can get the output what you are expecting but sometimes it won't give you the expected or even any assumed output. About the sequence points, that's another question :-) and moreover, the solution would become too complex for other readers if I will explain the sequence points. You can try googling, though. I am sure, you will find some interesting articles regarding these. — Abhineet, Mar 14 '14 at 13:43
Food for thought:: Why do you think that 'n' will be first decremented and then multiplied to 'facto(int)'? Why haven't you assumed that 'n' will first be multiplied and then 'decremented'? The answer is because, standards have not defined the order of evaluation in cases like this. Cases like what??? Cases that contain the multiple sequence points and their side effects. Follow the link for an overview:: http://www.geeksforgeeks.org/sequence-points-in-c-set-1/ — Abhineet, Mar 14 '14 at 13:51

Bathsheba · Answer 1 · 2014-03-14T12:54:14.333

12

Currently, by using n-- you are passing the original and therefore unmodified value of n into facto which is causing a loop.

You need to use n - 1 instead. It would, on the face of it, be tempting to use --n since that would decrement n and evaluate to the new (lower) value. But --n will give you undefined behaviour since you are pre-multiplying the function return value by n and since * is not a sequence point, the value of n is not well-defined.

(By the way, the behaviour in C would have been identical).

[Edit: acknowledge Mike Seymour on the undefined behaviour point].

edited Mar 14 '14 at 12:54

answered Mar 14 '14 at 12:31

Bathsheba

231,907
34
361
483

1

No, you don't need to use `--n`. That gives undefined behaviour here (since there's a second unsequenced access to `n`); and the value of `n` is unused after this, so there's no point in modifying its value anyway. It should be `n-1`. – Mike Seymour Mar 14 '14 at 12:42
@Paranaix Right, but is `n` (the left operand of the `*`) evaluated before or after the function call? – fredoverflow Mar 14 '14 at 12:46
I think (and could be wrong), that the function call ensures that the decrement happens before (i) the function call and (ii) the evaluation of `n` on the left side of `*`. – Bathsheba Mar 14 '14 at 12:51
@Bathsheba: No. It could, for example, evaluate `n` and `--n` with no sequencing, then call the function, then evaluate the result of `*`. – Mike Seymour Mar 14 '14 at 12:52
OK. I'll amend the answer. – Bathsheba Mar 14 '14 at 12:52
@Paranaix: Indeed, but it doesn't sequence the evaluation of its argument(s) with respect to any other operations. All it guarantees is that the function call is sequenced before the evaluation of `*`. – Mike Seymour Mar 14 '14 at 12:54
@MikeSeymour Imagine the multiplication happen before the function call :-D – fredoverflow Mar 14 '14 at 12:55
@Paranaix using --n will cause the `n` to decrement before the statement is executed. So, due to pre-decrement, the expression will become `(n-1)*facto(n-1)` . That is why it is better not to use pre-decrement in this case – ColorDeColor Mar 14 '14 at 13:10
@ColorDeColor: It might, or it might not. The evaluation order of the two operands is unspecified, so you might get that, or something else, or some other undefined behaviour. (But yes: it's definitely better not to use pre-decrement here.) – Mike Seymour Mar 14 '14 at 13:18

score 6 · Accepted Answer · edited May 23 '17 at 12:15

EDIT:: The explanation below is only to shed light on the usage of Post and Pre-Decrement for OP's better understanding of them. The correct answer for OP's code is, n*facto(n - 1). @OP: You should not do any pre-drecrement in that part of your code because it will invoke Undefined Behavior due to unsequenced modification of variable n.

Pre And Post-Decrement:: You have to use pre-decrement (wiki-link) if you want to decrement the variable before the value is passed. On the other hand, a post-decrement evaluates the expression before the variable is decremented:

int n = 10, x;
x = --n;                // both are 9

and

int n = 10, x;
x = n--;                // x = 10, i = 9

Why not to use pre-decrement in your case?:: n*facto(n--) causes UB. Why?

The Standard in §5/4 says

Between the previous and next sequence point a scalar object shall have its stored value modified at most once by the evaluation of an expression.

and

The prior value shall be accessed only to determine the value to be stored.

It means, that between two sequence points a variable must not be modified more than once and, if an object is written to within a full expression, any and all accesses to it within the same expression must be directly involved in the computation of the value to be written.

score 0 · Answer 3 · answered Mar 14 '14 at 12:33

0

return ( n*facto(n--) );

You are using the post decrement operator. What happens is, You pass the value of n to the function and then decrement it. That doesn't affect the value that is already passed to the function

answered Mar 14 '14 at 12:33

Suvarna Pattayil

5,136
5
32
59

Please look at other answers regarding 'undefined behavior' – Mar 14 '14 at 13:00
@DieterLücking: This is copying code from the question in order to describe why it's wrong, not suggesting that one should use this code. (The undefined behaviour is a second reason for it to be wrong, but not particularly relevant.) – Mike Seymour Mar 14 '14 at 13:04

Decrement Operator in C++

3 Answers3