I am new to Java and am I trying to implement a recursive method for finding "n choose k".
However, I've run into a problem.
public class App {
public static void main(String[] args) throws Exception {
int n = 3;
int k = 2;
int result = combRecursion(n, k);
System.out.println(result);
}
private static int combRecursion(int n, int k) {
if (k == 0) {
return 1;
} else {
return (combRecursion(n - 1, k - 1) combRecursion(n - 1, k));
}
}
Output: many repetitions of this line:
at App.combRecursion(App.java:14)
CodePudding user response:
It's possible to pick k
items from the set of n
items only if n
is greater or equal to k
.
You need to cut off fruitless branches of recursion spawn by the call combRecursion(n - 1, k)
which doesn't reduce the number of item in the sample.
When you need to create a recursive method, it should always contain two parts:
Base case - that represents a set of edge-cases, trivial scenarios for which the result is known in advance. If the recursive method hits the base case (parameters passed to the method match one of the conditions of the base case), recursion terminates. In for this task, the base case will represent a situation when the source list was discovered completely and position is equal to its size (invalid index).
Recursive case - a part of a solution where recursive calls are made and where the main logic resides.
Your recursive case is correct: it spawns two recursive branches of execution (one will "pick" the current item, the second will "reject" it).
But in the base case, you've missed the scenario mentioned above, we need to address these situations:
n
isn't large enough (k > n
), so that is not possible to fetchk
item. And the return value will be0
(or instead of returning a value, you might throw an exception).k == 0
result should be1
(it's always possible to take0
items, and there's only one way to do it - don't pick anything).- When
k == n
- there's only one way to construct a combination, as @akuzminykh has pointed out. And the return value will be1
Note that because your goal is to get familiar with the recursion (I'm pretty sure that you're doing it as an exercise) there's no need to mimic the mathematical formula in your solution, use pure logic.
Here is how you can implement it:
private static int combRecursion(int n, int k) {
if (k > n) return 0; // base case - impossible to construct a combination
if (n == k || k == 0) return 1; // base case - a combination was found
// recursive case
return combRecursion(n - 1, k - 1) combRecursion(n - 1, k);
}
main()
- demo
public static void main(String[] args) {
System.out.println(combRecursion(3, 2));
System.out.println(combRecursion(5, 2));
}
Output
3 // pick 2 item from the set of 3 items
10 // pick 2 item from the set of 5 items
CodePudding user response:
Your base case ought to include both n == k || k == 0
for "n choose k" to be implemented correctly. That way, both calls will eventually terminate (even though your current implementation is rather inefficient as it has exponential runtime). A better implementation would use the formula n!/k!/(n-k)!
or the multiplicative formula to run in linear time:
int factorial(int n) {
int res = 1;
for (; n > 1; n--) {
res *= n;
}
return res
}
int choose(int n, int k) {
return factorial(n)/factorial(k)/factorial(n-k);
}
further optimizing this is left as an exercise to the reader (hint: a single for loop suffices).
CodePudding user response:
Within your recursive method you've written that k
is the parameter which will determine the base case to end the recursion.
Your base case only checks k
s value and your right recursive call keeps making a recursive invocation where k
stays the same and its nested recursive invocations will do as much in turn, endlessly producing right recursive calls with no end.
That recursive call is what is giving you a StackOverflowError
.
You should rewrite it as:
int choose(int n, int k) {
if (k == 0) return 1;
return (n * choose(n - 1, k - 1)) / k;
}