# Is this piece of code logically correct???

This is a discussion on Is this piece of code logically correct??? within the C++ Programming forums, part of the General Programming Boards category; Cod e to find factorial... ignoring any syntax errors (plz tell me if any) is it logically correct? Code: #... ...

1. ## Is this piece of code logically correct???

Cod e to find factorial... ignoring any syntax errors (plz tell me if any) is it logically correct?

Code:
```#...
#...

int fact(int a);

void main()
{
...
...
...
...
cin >> a;
y = fact(a);
cout << y;
...
...
...
}

int fact(int a)
{ if (a==1)
return(1);
return  ( a * fact(a-1)
}```

2. Yeah - close enough - now try it with int main() and see how you get on.

3. Code:
```#...
#...

int fact(int a);

INT main()
{
...
...
...
...
cin >> a;
y = fact(a);
cout << y;
...
...
...
}

int fact(int a)
{ if (a==1)
return(1);
return  ( a * fact(a-1)
}```
What i actually wanted to kmow, was if there is a bretter wway to get this program done???

From now on, i use int main().

4. Your factorial code is incorrect.

0! is defined and is equal to 1.

5. Thanks a lot Rahakil Fol! for correcting me...
but otherwise is it fine??

And fopr any other reason, could there be a better way to do it?

-pritin

6. Code:
`return  ( a * fact(a-1) );`

7. There are other ways of finding the factorial, including non-recursive techniques. Whether they are better depends on the situation.

8. Your fact function will also be infinitely recursive if given a negative argument. In practice, that probably means you'll get your program terminating in a strange way. Try using an unsigned argument.

Also keep in mind that it won't take a particularly large value to give an integer overflow when computing factorial.

9. Originally Posted by grumpy
Your fact function will also be infinitely recursive if given a negative argument. In practice, that probably means you'll get your program terminating in a strange way. Try using an unsigned argument.
Using an unsigned argument won't really help matters. It'll end up recursing just as much if you give the unsignedly-defined function a negative argument, or if you give it an unsigned integer that has been treated in ways for which, were it considered signed by the compiler, its bits would make it negative. (In other words, really large.)

Originally Posted by Daved
Whether they are better depends on the situation.
Non-recursive techniques for computing the factorial are "always" better than recursive, unless they're written very poorly.

Unless the compiler is smart enough to notice that multiplication is associative and has an identity element and is willing to use this to optimize the recursive code away.

10. >> Using an unsigned argument won't really help matters.

It would help matters by indicating to the caller that you expect a positive value. It would be the caller's responsibility to make sure a negative number is not passed to the function.

11. Congratulations, you have successfully equivocated the context of this discussion's sentences into something else.

12. Well, since negative values are illegal, it does make sense to use an unsigned argument despite the fact that a large unsigned argument could overflow (which, there are ways to detect and report an error, but at this point, since that is somewhat tangential to what you are learning, I'd ignore). Additionally, an unsigned value will possibly* allow you to correctly (without overflow) compute larger factorials since you have an extra "useful" power of 2.

Non-recursive techniques for computing the factorial are "always" better than recursive, unless they're written very poorly.
Well, if you knew what you needed at compile-time, and you didn't feel like pulling out a calculator, you could use recursive techniques to make the compiler do the work for you.

* Whether it does or not depends on the size and how the values fall. It (likely) won't make a difference.

13. Unsigned values _are_ good to use for this function. They just don't happen to change the behavior with invalid input. The return value should of course be unsigned.

Unsigned values would also be useful if you wanted to check for input being in a valid range -- that is, to check and make sure that the passed argument is small enough so that the return value is not overflown. Then, instead of checking (a <= max_arg && a >= 0), you can just test (a <= max_arg).

14. Originally Posted by Rashakil Fol
Unsigned values _are_ good to use for this function. They just don't happen to change the behavior with invalid input. The return value should of course be unsigned.
That's not quite true. A large positive value, regardless of whether it is of unsigned type or not, will cause an overflow.

In the code as given in the original post (where the argument and return type was an int), a negative value will result in infinite recursion (each call of fact(n) will call fact(n-1) if n is negative).

If a negative value is passed to a function expecting an unsigned argument, the result is implementation defined (as the result of converting a negative int to unsigned int is implementation defined). In practice (with a lot of compilers) negative values, when converted to unsigned, will result often result in a large value, but such behaviour is not mandated by wither C or C++ standards.