-1

I have some method to find a factorial of big numbers. Could somebody explain, what's wrong with it and why i haven't any output?

public static long factorial(long num) {
    BigInteger numm = BigInteger.valueOf(num);
    BigInteger fact= BigInteger.valueOf(1);
    for (; numm.compareTo(BigInteger.ZERO)==1 ; fact = fact.multiply(numm)) {
        numm.subtract(BigInteger.ONE);
    }
    return fact.longValue();
}
user3545646
  • 21
  • 2
  • You discard the result of your subtraction operation (notice how you wrote `fact = fact.multiply(numm)` to update `fact` after the multiplication operation?) That means `numm` will always be the same value, and the loop will continue "forever". –  Sep 26 '15 at 13:07
  • possible duplicate of [Find factorial of large numbers in Java](http://stackoverflow.com/questions/11446973/find-factorial-of-large-numbers-in-java) – Joe Sep 27 '15 at 08:59

2 Answers2

0

I don't think that's how you write a factorial. And why do you use BigInteger when you are returning a long? So just make the decision, long or BigInteger. I would choose BigInteger because you said you want to operate on very large numbers. You should use recursion to do factorial.

public BigInteger factorial (BigInteger number) {
    if (number.equals(BigInteger.ONE) || number.equals(BigInteger.ZERO)) {
        return BigInteger.ONE;
    }
    return number.multiply(factorial(number.subtract(BigInteger.ONE)));
}
Sweeper
  • 213,210
  • 22
  • 193
  • 313
  • Factorial is a good example to show recursion basics, but in general if there's a so nice algorithm which doesn't require recursion, why should we use it? – Vitaly Zinchenko Sep 26 '15 at 14:09
0

You don't assign the subtraction value to numm. This is the problem. To stay with your code, use num + 1 because last part of the for loop execute after the substraction made. So, need one extra iteration.

Check that:

long num=5;
BigInteger numm = BigInteger.valueOf(num + 1);
BigInteger fact= BigInteger.valueOf(1);
for (; numm.compareTo(BigInteger.ONE)==1 ; fact = fact.multiply(numm)) {
    numm = numm.subtract(BigInteger.ONE);
}
System.out.println(fact.longValue());
Animesh Kumar Paul
  • 2,241
  • 4
  • 26
  • 37