Not Yet Answered # Please review factorial code

Schol-R-LEA 1,003 Discussion Starter Raisefamous -3 NathanOliver 429 dinamit3 WaltP 2,905 Schol-R-LEA 1,003 firstPerson 761 WaltP 2,905 vijayan121 1,152 esesili Discussion Starter Raisefamous -3 Hey, so I wanna ask how I need to create a method who will remove word if in that word is 2 same chars. Example: "Potato" in this word there is a 2 "o" chars so this word will need to be removed. "Forum" in this word there is no ...

0

Write a program which finds the factorial of a number entered by the user. (check for all conditions) (Beginner).

Hi,

My knowledge about C++ and programming so far is 27 youtube videos. It took me 1,5 hours to figure out and write this :)

```
int main()
{
int x = 1;
int number;
cin >> number;
int factorial = number;
while (x != number){
factorial = factorial * (number - x);
x++;
}
cout << "Factorial is " << factorial << endl;
}
```

*Edited 4 Years Ago by ~s.o.s~*: Added code tags, learn to use them.

0

While it isn't necessarily how I would do it, it does seem to be a valid approach to the problem. The only thing I see wrong with it is that you need to add the header for `<iostream>`

, and a `using namespace std;`

directive.

```
#include <iostream>
using namespace std;
```

If you add these at the beginning of the program, it should compile correctly and run.

Have you tried running it yet? Which compiler are you using, and do you need any help with using it correctly?

0

Hi,

Yes i have added derective and the library, just forgot to add them here, programm runs perfect. I'm using Codeblocks.

What is your option for doing this problem?

0

If I was writing a code to compute the factorial of a number I would do this.

```
#include <iostream>
using std::cout;
using std::cin;
int main()
{
int factorial = 1, number;
cout << "Please enter a number to find its factorial: ";
cin >> number;
for (int i = 2, i <= number; i++) // loop through all of the numbers
{
factorial *= i;
}
cout << "The factorial of " << number << " is: " << factorial;
return 0;
}
```

0

Raisefamous you should see function and recursive implementation.

Look how easy is recursive implementation :

```
double factorial (int n)
{ if(n==0)
return 1;
else
return n*factorial(n-1);
}
```

*Edited 4 Years Ago by dinamit3*: n/a

0

Raisefamous you should see function and recursive implementation.

Look how easy is recursive implementation :`double factorial (int n) { if(n==0) return 1; else return n*factorial(n-1); }`

Look how easy a nonrecursive solution is:

`for (int i = 2, i <= number; i++) // loop through all of the numbers { factorial *= i; }`

2 lines fewer...

And not a memory hog.

0

And not a memory hog.

Seriously? True, the iterative version of factorial is O(1) in memory, whereas recursive factorial is O(n) in memory. Assuming that a stack frame is, 32 bytes (assuming a 64-bit `unsigned long`

, 64-bit memory addressing, and no register painting) that would amount to 8KiB for 256! (even if you could, with a 64 bit value). How often do you really compute 256! ? Even on an embedded system, that is hardly a memory hog.

0

>>Even on an embedded system, that is hardly a memory hog.

How about calculating 100! or 1000!, putting aside the limitation of data-type of course. Anyways, usually tail recursion can be removed by compiler's optimization.

OP have you learned about functions? Try to modularize your code by using functions.

```
//given a positive natural number n, it returns the factorial
//of n, that is it returns n*(n-1)*...*(2)*(1)
unsigned int factorial(int n){
unsigned int fact = n;
while(n --> 1){ fact *=n; }
return fact;
}
int main(){
unsigned int input = 0;
cin >> input ;
cout << factorial( input ) << endl;
}
```

Also don't forget to comment on your code. And try to indent your code for better visualization. Other than that, good job!

*Edited 4 Years Ago by firstPerson*: n/a

0

Seriously? True, the iterative version of factorial is O(1) in memory, whereas recursive factorial is O(n) in memory.

I'm soooo sorry. Let me rephrase:

"*2 lines fewer... (3 if you formatted better)
And not a wasting O(n) memory as well as n calls and returns. *"

Better? :-P

0

> My knowledge about C++ and programming so far is 27 youtube videos.

> It took me 1,5 hours to figure out and write this

Pretty good first effort, I would say. Well done!

Relying on youtube videos alone might not be a good idea; get a text book too.

'Accelerated C++' by Koenig and Moo, for instance. I like it because it teaches C++ from the outset (rather than how to compile C code using a C++ compiler). http://www.acceleratedcpp.com/

> Look how easy a nonrecursive solution is:

Depends on who is doing the looking. A lisper would say: 'Look how easy a recursive solution is'.

> "2 lines fewer... (3 if you formatted better)

Really?

```
int factorial_recursive( int n )
{
return n==1 ? 1 : n * factorial_recursive(n-1) ;
}
int factorial_iterative( int n )
{
int result = 1 ;
for( int i = 2 ; i <= n ; ++i ) result *= i ;
return result ;
}
```

Depends on who writes the code; and how it is written.

> And not a wasting O(n) memory as well as n calls and returns. "

> Better? :-P

Not any better; while being more pompous, it is still based on the same naive assumption that it is the C++ source code that gets executed directly at run time.

I just tried to actually verify just how wasteful the recursive version could be; here are the results:

Using g++ 4.7 with :**>g++ --std=c++0x -Wall -Werror --pedantic --pedantic-error -Wextra -O3 -fomit-frame-pointer -c -S test_it.cc**

The iterative version:

```
int factorial_iterative( int n )
{
int result = 1 ;
for( int i = 2 ; i <= n ; ++i ) result *= i ;
return result ;
}
```

generated this code:

```
__Z17factorial_iterativei:
movl 4(%esp), %ecx
cmpl $1, %ecx
jle L10
addl $1, %ecx
movl $2, %edx
movl $1, %eax
L9:
imull %edx, %eax
addl $1, %edx
cmpl %ecx, %edx
jne L9
rep
ret
L10:
movl $1, %eax
ret
```

And the recursive version:

```
int factorial_recursive( int n )
{
return n==1 ? 1 : n * factorial_recursive(n-1) ;
}
```

generated:

```
__Z19factorial_recursivei:
movl 4(%esp), %edx
movl $1, %eax
cmpl $1, %edx
je L4
L3:
imull %edx, %eax
subl $1, %edx
cmpl $1, %edx
jne L3
rep
L4:
ret
```

It turns out that at least with one compiler, but the iterative version is more wasteful than the recursive version. Tail-call recursion strikes again!

*Edited 4 Years Ago by vijayan121*: n/a

0

Here is another factorial program. Almost same with others

```
#include <iostream>
using namespace std;
int main()
{
long double n;
long double result = 1;
int k;
cout << "please enter a non-negative number"<< endl;
cin>> n;
if (n<0)
{
cout<<"the number is less then 0."<< endl;
cout<<"please enter a non-negative number" << endl;
cin>>n;
for (k = 2; k<=n; k++)
result = result*k;
}
else
{
for (k = 2; k<=n; k++)
result = result*k;
}
```

0

Hi guys,

Just a quick question. Is "factorial *= i;" the same as "factorial = factorial * i; ?

and also "sum += tuna[x]" the same as "sum = sum + tuna[x]" ?

Thanx

*Edited 4 Years Ago by Raisefamous*: n/a

This article has been dead for over six months. Start a new discussion instead.

Recommended Articles

Hi. I have a form with list box : lst_product, datagridview : grd_order and button: btn_addline. lst_product has a list of product ids selected from database (MS Acess 2013) , grd_order is by default empty except for 2 headers and btn_addline adds rows to grd_order.

btn_addline :

`Private Sub btn_addline_Click(ByVal ...`

I don’t want at this stage work on a big separate project as I've already got plenty ...