Question

... 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?

Was it helpful?

Solution

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;

OTHER TIPS

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.

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;
}

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;
}

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;

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;
}
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top