One way is to do caching, which comes with the great price of memory usage.
public static int combinations(int group, int members) {
if (members > group - members) {
members = group - members; // 21 choose 17 is same as 21 choose 4
}
final int[][] cache = new int[group][members];
return combinations(group, members, cache);
}
private static int combinations(int group, int members, int[][] cache) {
if (cache[group - 1][members - 1] > 0) {
return cache[group - 1][members - 1];
}
else if (members == 1) {
cache[group - 1][members - 1] = group;
return group;
}
else if (members == group) {
cache[group - 1][members - 1] = 1;
return 1;
}
else {
return (combinations(group - 1, members - 1, cache) + combinations(group - 1, members, cache));
}
}
I did some quick test (non-professional benchmarks) and found that the original method takes half the time of the caching method. Looks like all these read/writes to the array cache is slowing things down drastically.
The other way is to change the whole formula.
public static int combinations(int group, int members) {
if (members > group - members) {
members = group - members;
}
int answer = 1;
for (int i = group; i > group - members; i--) {
answer *= i;
}
for (int i = 1; i <= members; i++) {
answer /= i;
}
return answer;
}
Again, I tested the new method with the original (which I made them use BigInteger for the test), and the new method is incredibly faster (26 seconds for original method vs 0.00 seconds for the latter for 35 choose 15).
To add on a bit, I think the time complexity for using recursive call is O((group)(log members)), while using the new formula is simply O(members).