I am new to Java and am I trying to write a method that will choose k from n.

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 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, there should be two cases:

• n isn't large enough, so that is not possible to fetch k item. Return value will be 0.
• k = 0 result should be one (it's always possible to take 0 items, and there's only one way to do it - don't pick anything).

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
    if (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 ks 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;
}