3

... preferably in Java. Here is what I have:

//x choose y
public static double choose(int x, int y) {
    if (y < 0 || y > x) return 0;
    if (y == 0 || y == x) return 1;

    double answer = 1;
    for (int i = x-y+1; i <= x; i++) {
        answer = answer * i;
    }
    for (int j = y; j > 1; j--) {
        answer = answer / j;
    }
    return answer;
}

I'm wondering if there's a better way of doing this?

echoblaze
  • 11,176
  • 13
  • 44
  • 49
  • As the choose function is defined as a binary function on natural numbers (|N x |N => |N), you should not use doubles at any point. This actually renders your chosen solution to be unnecessarily suboptimal. – Ralph M. Rickenbach May 28 '10 at 13:46

6 Answers6

8
choose(n,k) = n! / (n-k)! k!

You could do something like this in O(k):

public static double choose(int x, int y) {
    if (y < 0 || y > x) return 0;
    if (y > x/2) {
        // choose(n,k) == choose(n,n-k), 
        // so this could save a little effort
        y = x - y;
    }

    double denominator = 1.0, numerator = 1.0;
    for (int i = 1; i <= y; i++) {
        denominator *= i;
        numerator *= (x + 1 - i);
    }
    return numerator / denominator;
}

EDIT If x and y are large, you will overflow more slowly (i.e., be safe for larger values of x & y) if you divide your answer as you go along:

    double answer = 1.0;
    for (int i = 1; i <= y; i++) {
        answer *= (x + 1 - i);
        answer /= i;           // humor 280z80
    }
    return answer;
mob
  • 117,087
  • 18
  • 149
  • 283
  • -1: The answer loses precision unnecessarily. choose(9,4) gets a multiplication by 7.0/3, which can't be expressed precisely in a double. See mine for a correct implementation. – Sam Harwell Nov 07 '09 at 19:42
  • Really? The correct implementation is to coerce two longs into `BigInteger` objects, to call `BigInteger.GreatestCommonDivisor`, and then to coerce the result back to a `long`? Seriously? – mob Nov 08 '09 at 00:43
  • *First* make yours return a correct result, *then* we can talk about which one is more efficient. – Sam Harwell Nov 08 '09 at 04:46
  • 1
    @280Z28 Don't be so full of yourself. `choose(11,5) => 461.99999999999994` with this algo, `choose(11,5) => 2184` with yours. But I get the wrong answer 10 times as fast. – mob Nov 10 '09 at 04:51
  • Why did you use floating point? In the edited answer, the variable `answer` is never a floating point value. Also, if you looped the numerator from small to large, you would have less chance of overflow. – Eyal Nov 05 '13 at 15:23
6

What you've got looks pretty clear to me, to be honest. Admittedly I'd put the return statements in braces as that's the convention I follow, but apart from that it looks about as good as it gets.

I think I'd probably reverse the order of the second loop, so that both loops are ascending.

As Greg says, if you need to get accurate answers for large numbers, you should consider alternative data types. Given that the result should always be an integer, you might want to pick BigInteger (despite all the divisions, the result will always be an integer):

public static BigInteger choose(int x, int y) {
    if (y < 0 || y > x) 
       return BigInteger.ZERO;
    if (y == 0 || y == x) 
       return BigInteger.ONE;

    BigInteger answer = BigInteger.ONE;
    for (int i = x - y + 1; i <= x; i++) {
        answer = answer.multiply(BigInteger.valueOf(i));
    }
    for (int j = 1; j <= y; j++) {
        answer = answer.divide(BigInteger.valueOf(j));
    }
    return answer;
}
MJBZA
  • 4,796
  • 8
  • 48
  • 97
Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
6

The numbers you are dealing with will become quite large and will quickly exceed the precision of double values, giving you unexpectedly wrong results. For this reason you may want to consider an arbitrary-precision solution such as using java.math.BigInteger, which will not suffer from this problem.

Greg Hewgill
  • 951,095
  • 183
  • 1,149
  • 1,285
1

I coded this in C#, but I tried to make it as applicable as possible to Java.

Derived from some of these sources, plus a couple small things from me.

Code:

public static long BinomialCoefficient(long n, long k)
{
    if (n / 2 < k)
        return BinomialCoefficient(n, n - k);

    if (k > n)
        return 0;

    if (k == 0)
        return 1;

    long result = n;
    for (long d = 2; d <= k; d++)
    {
        long gcd = (long)BigInteger.GreatestCommonDivisor(d, n);
        result *= (n / gcd);
        result /= (d / gcd);
        n++;
    }

    return result;
}
Sam Harwell
  • 97,721
  • 20
  • 209
  • 280
  • This code is strange. Why does n keep increasing? And why not derive n from d instead of modifying it in the loop? – Eyal Nov 05 '13 at 15:20
1

for

N!/((R!)(N-R)!)

use this (Pseudocode)

if (R>N) return 0;

long r = max(R, N-r)+1;
if (R==N) return 1;

for (long m = r+1, long d = 2; m <= N; m++, d++ ) {
    r *= m;
    r /= d;
}
return r;
Ralph M. Rickenbach
  • 12,893
  • 5
  • 29
  • 49
0

This version does not require BigInteger or floating-point arithmetic and works without overflow errors for all n less than 62. 62 over 28 is the first pair to result in an overflow.

public static long nChooseK(int n, int k) {
    k = Math.min(k, n - k);

    if (n < 0 || k < 0)
        throw new IllegalArgumentException();

    if (k == 0)
        return 1;

    long value = n--;

    for (int i = 2; i <= k; i++) {
        value = Math.multiplyExact(value, n--);
        value /= i;
    }

    return value;
}

The following test proves that this is true:

@Test
void nChooseKLongVsBigInt() {
    for (int n = 0; n < 62; n++) {
        for (int k = 0; k <= n; k++) {
            assertEquals(nChooseKBigInt(n, k), BigInteger.valueOf(nChooseK(n, k)));
        }
    }
}

private BigInteger nChooseKBigInt(int n, int k) {
    return factorial(n).divide(factorial(k).multiply(factorial(n - k)));
}

private BigInteger factorial(int number) {
    BigInteger result = BigInteger.ONE;

    for (int factor = 2; factor <= number; factor++) {
        result = result.multiply(BigInteger.valueOf(factor));
    }

    return result;
}
devconsole
  • 7,875
  • 1
  • 34
  • 42