2

I am a beginner in c++ and I am having problems with making this code work the way I want it to. The task is to write a program that multiplies all the natural numbers up to the loaded number n.

To make it print the correct result, I divided x by n (see code below). How can I make it print x and not have to divide it by n to get the correct answer?

#include<iostream>
using namespace std;
int main(){
    int n,x=1;
    int i=0;
    cout<<"Enter a number bigger than 0:"<<endl;
    cin>>n;
    while(i<n){
        i++;
        x=i*x;  
    };
    cout<<"The result is: "<<x/n<<endl;
    return 0;
}
anatolyg
  • 26,506
  • 9
  • 60
  • 134
Binary
  • 27
  • 1
  • 5

3 Answers3

5

At very first a principle you best get used to as quickly as possible: Always check user input for correctness!

cin >> n;
if(cin && n > 0)
{
    // valid
}
else
{
    // appropriate error handling
}

Not sure, why do you need a while loop? A for loop sure is nicer in this case:

int x = 1;
for(int i = 2; i < n; ++i)
   x *= i;

If you still want the while loop: Start with i == 2 (1 is neutral anyway) and increment afterwards:

i = 2;
while(i < n)
{
    x *= i;
    ++i;
}

In case of n == 1, the loop (either variant) simply won't be entered and you are fine...

Aconcagua
  • 24,880
  • 4
  • 34
  • 59
  • I would rather have `if (!cin || n < 0)` and error handling instead of `if`/'else` –  Mar 22 '18 at 21:27
2

You already have two very good options, but here is an other one you might want to take a look at when you are at ease enough in programming :

unsigned factorial(unsigned value)
{
    if (value <= 1) 
    {
        return 1;
    }
    else
    {
        return value * factorial(value - 1);
    }
}

It's a recursive function, which is kind of neat when used in proper moments (which could not be the case here unfortunately because the execution stack might get so big you fill your memory before you're done. But you can check it out to learn more about recursive functions)

When your memory is full, you then crash your app with what is called actually a stack overflow.

Sirmyself
  • 1,464
  • 1
  • 14
  • 29
  • ... unless [tail call optimised](https://stackoverflow.com/questions/310974/what-is-tail-call-optimization)... – Aconcagua Mar 22 '18 at 22:02
  • @Aconcagua Yes, but this example is not and is a simpler one to both understand recursivity and stack overflow risks associated to it – Sirmyself Mar 23 '18 at 12:37
  • Did not intend to criticise (actually, I was one of the at the time being two upvoters...) but to provide *additional* info... Given [answer](https://stackoverflow.com/a/9814654/1312382) explains quite well why not and what to do to get it... By the way, actually, we calculate the `factorial`, although exponential could easily be calculated recursively as well (`return base * exponential(base, exp - 1);`)... – Aconcagua Mar 23 '18 at 15:56
  • my bad : I just remembered incorrectly (I corrected my answer) – Sirmyself Mar 23 '18 at 15:57
  • For correctness: you might return 1 on `value <= 1`, as `0!` is 1, too... Would we want to return 0 for negative values to indicate illegal input (wouldn't have been an issue with unsigned int...)? As C++, we might throw appropriate exception, too... – Aconcagua Mar 23 '18 at 16:00
  • I thought 0! was 0, but I mixed up with summation rules. For negative values, I usually use those as error detection which is why I just return values lower than 0 – Sirmyself Mar 23 '18 at 17:11
  • Hehe, then we can get it easily: `return value | (value == 0);` – Aconcagua Mar 23 '18 at 17:12
  • I did pretty much that haha I changed types so no negative values can get in the function. validation must now be made at data entry instead of the function itself – Sirmyself Mar 23 '18 at 17:14
1

How can I make it so that in the last cout I can only put x and not have to divide x by n to get the correct answer?

It will be better to use a for loop.

// This stops when i reaches n. 
// That means, n is not multiplied to the result when the loop breaks.
for (int i = 1; i < n; ++i )
{
   x *= i;
}

cout << "The result is: " << x <<endl;
Waqar
  • 8,558
  • 4
  • 35
  • 43
R Sahu
  • 204,454
  • 14
  • 159
  • 270