Find factorial Using call by reference function in C++

Question

I am missing very minor thing here i could not figure it out, i am just try to get the factorial of the number inputted, using a recursive call by reference concept in C++. Please let me know what am i missing. Declaring few additional variables for more further alterations in programs.

#include <iostream>
using namespace std;
int factorial(int &m);
int factorial(int &m)
{
    int i,result=0;
    
    
    if(m>=1)
    return  m * factorial(m-1);
    else
        return 0;

}

int main(){

    int m,i,factorial1=1;
    
    cout<<"Please enter the number to factor   "<<std::endl;
    cin>>m;
    factorial1=factorial(m);

    cout<<"Factorial value is " << factorial1<<std::endl;
    return 0;
    }

Getting error as below

factorial_func.cc: In function ‘int factorial(int&)’:
factorial_func.cc:33:25: error: cannot bind non-const lvalue reference of type ‘int&’ to an rvalue of type ‘int’
   33 |  return m * factorial((m-1));
      |                       ~~^~~
factorial_func.cc:27:20: note:   initializing argument 1 of ‘int factorial(int&)’
   27 | int factorial(int &m)
      |               ~~~~~^

The result of `m - 1` is an *rvalue*, and lvalue-references (which `int &m` is) can't be bound to such values. Use e.g. `const int &m` instead. Or since `m` isn't modified, just pass it by value instead. — Some programmer dude, Jun 22 '20 at 18:37
Another problem: if `m == 0`, the function should return 1, not 0 — Damien, Jun 22 '20 at 18:41

Rohan Bari · Accepted Answer · 2020-06-22T19:34:41.030

The errors in your program:

It tries to bind a non-constant lvalue (roughly it means a variable that could be assigned in the left hand side of = sign) reference of type an integer reference (from argument) to an rvalue of an integer in the syntax:
```
return  m * factorial(m - 1);
```
Passing a reference is not required here at all. You may remove &.
The else statement returns a zero instead of one when the m is lesser than 1 and that gives you unexpected output since any multiplication operation with a zero is always a zero.

You should try:

Returning an integer from the argument directly rather than returning a reference value.
You must return 1 instead of 0 when m reaches lesser than 1 in the else condition.
The variables declared in factorial() are totally unused and redundant and i is unused in main().

Code redefined:

#include <iostream>

int factorial(int param) {
    if (param >= 1)
        return param * factorial(param - 1); // new * (-1 than new) recursively
    else
        return 1;
}

int main(void) {
    int fact = 1;
    int num;

    std::cout << "Please enter the number to factor: ";
    std::cin >> num;

    fact = factorial(num);

    std::cout << "Factorial is: " << fact << std::endl;

    return 0;
}

After fixing the errors, you'll get output like:

$ g++ -o prog prog.cpp; ./prog
Please enter the number to factor: 5 // --- INPUT
Factorial is: 120 // --- OUTPUT

Lois · Answer 2 · 2020-06-24T17:41:50.293

Dude's answer is correct, there is no need to apply an lvalue ref as argument, a pass by (r)value is correct, this applies as general advice for recursive functionality to avoid side effects. More serious is a fault: factorial of 0 is not 0, but by definition it is 1 !!! Whether this sounds illogical, see the discussion on https://www.quora.com/Why-does-zero-factorial-0-equal-one-1-1

@Dorian: It's easy to spare 2 recursive calls by tackling the factorials of 0 1 2:

int factorial(int m)
{
    if (m > 2)
        return  m * factorial(m - 1);
    else if (m == 0)
        m = 1;
    return m;
}

Dorian · Answer 3 · 2020-06-23T09:07:34.267

Here's your working code. Some tipps:

why not use using namespace std
You need to pass 1 in the last factorial, otherwise your result is multiplied by 0 and that's 0, so it won't work
if you only pass single ints, you can pass them easier by value (gives you less problems in the beginning). BUT: in any case passing by reference here is wrong, because you don't want the original value to change, but in recursion you want to have a RETURN value.
regarding you original error: you are trying to bind an rvalue (a value that stands ONLY on the right side of an equation) to an lvalue reference (&m), which you cannot do, since if you are changing the lvalue reference, you cannot change the right side of the equation. You can use rvalue references (&&m), but this is quite advance C++ and you should maybe read up here first to get an idea of what you are dealing with.

#include <iostream>
int factorial(int m);
int factorial(int m)
{    
    if (m >= 1)
       return  m * factorial(m-1);
    else
       return 1;

}

int main(){

    int m, factorial1 = 1;
    
    std::cout << "Please enter the number to factor" <<std::endl;
    std::cin >> m;
    factorial1=factorial(m);

    std::cout<<"Factorial value is " << factorial1 << std::endl;
    return 0;
}

Find factorial Using call by reference function in C++

3 Answers3