-4

Good day! I am having some trouble with my permutation calculator. For some reason, the end result is always 1. We are not allowed to use recursion at the moment so I opted to use a for loop for finding the factorials.

Here is my code:

#include <iostream>
using namespace std;

int fact(int x);
int perm(int y, int z);
int n, r, npr;
char ch;


int main()
{

    do{

        cout<<"Enter n (object/s): ";
        cin>> n;
        cout<< "Enter r (sample/s): ";
        cin>> r;


        npr= perm(n,r);

        cout<< "Value of "<< n<<"P"<< r<< " = "<< npr<<endl;
        cout<<"Would you like to repeat this again? [y/n] \n";
        cin>> ch;
        cout<< "\n";

    } while(ch=='y');

    cout<< "Thank you and have a nice day!";

    return 0;
}


int fact(int x)
{
   int number, cum = 1;

    for(number=1;number<=n;number++)
      cum=cum*number;

    return cum;
}

int perm(int y, int z)
{

    return fact(n) / fact(n-r);
}
Chadwick Macavoy
  • 1
  • 2
  • 1
    don't use globals variables. I don't know if they responsible for the wrong output, but they are responsible for your code being difficult to read and understand. It is very confusing that the functions have arguments that are unused – 463035818_is_not_an_ai Jan 17 '22 at 11:37
  • 3
    It is *very* odd that your functions aren't using their parameters for anything. If you print the results of calling `fact`, you will see that they are the same number. – molbdnilo Jan 17 '22 at 11:40
  • `fact` always returns `fact(n)`. `fact(n)/fact(n)` is 1, which is what you are getting. – Daniel Langr Jan 17 '22 at 11:43
  • Thank you for the responses! I apologize for my mistake as a beginner. However, I'd just like to clarify, should I not be using the function prototype thingies at the top when using functions? Also, should the parameters inside the headers of the function definition part be the variables that names of the variables I actually use? I apologize if this seems like a stupid question. @463035818_is_not_a_number – Chadwick Macavoy Jan 17 '22 at 11:44
  • 1
    Read a little bit about functions and their parameters, and how to use them, in your favourite C++ book. – molbdnilo Jan 17 '22 at 11:46

1 Answers1

0

The problem in your code is unecessary abuse of global variables. This function:

int fact(int x) 
{
   int number, cum = 1;

    for(number=1;number<=n;number++)
      cum=cum*number;

    return cum;
}

Always calculates the factorial of n. No matter what parameter you pass when calling it, hence here:

int perm(int y, int z)
{

    return fact(n) / fact(n-r);
}

fact(n) returns the factorial of n. fact(n-r) returns the factorial of n. And the result is always 1. Remove the globals and make the functions acutally use their arguments:

#include <iostream>

int fact(int x);
int perm(int y, int z);


int main() {

    int n = 0;
    int r = 0;
    char ch = 'n';

    do{    
        std::cout << "Enter n (object/s): \n";
        std::cin >> n;
        std::cout << "Enter r (sample/s): \n";
        std::cin >> r;
        auto npr = perm(n,r);
        std::cout << "Value of "<< n << "P" << r << " = " << npr << "\n";
        std::cout << "Would you like to repeat this again? [y/n] \n";
        std::cin >> ch;
        std::cout << "\n";  
    } while(ch=='y');

    std::cout << "Thank you and have a nice day!";
}

int fact(int x) {
   int cum = 1;   
   for(int number=1;number<=x;number++) {
      cum=cum*number;
   }
   return cum;
}

int perm(int y, int z) {
    return fact(y) / fact(y-z);
}
463035818_is_not_an_ai
  • 109,796
  • 11
  • 89
  • 185
  • Thank you very much! I have one last question if it's alright. What does the std:: do? Again, I apologize if this is a stupid question. – Chadwick Macavoy Jan 17 '22 at 12:13
  • @ChadwickMacavoy see here [Why is “using namespace std;” considered bad practice?](https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice) – 463035818_is_not_an_ai Jan 17 '22 at 12:14