Making recursion optional: Bad practice?

Question

Let's say I have a function like this:

/*
Mode is a bool that determines whether the function uses 
it's recursive mode or not
*/
public static int func(int n, boolean mode) {
    if (!mode) someCalculation();
    else func(anotherCalculation(), mode);
}

That is to say, depending on what mode is, the same function decides to either do a recursive calculation or a non-recursive one, where the two calculations are not related but must be done to the same datum. Under any circumstances would doing this be a better practice than simply having one function call two different functions that do the two jobs?

Answer 1

The rest of the answers are spot-on generally. I would only add that, depending on the context where you need this function, or any function that is "dichotomous" under your perspective (I mean, recursive or not is not always a concern in the majority of calculation cases in the real world, but it seems to matter to you), you may want to actually hide the implementation behind an abstraction: (using C# here)

public interface IMathCalculator
{
    //....
    int Factorial(int n);
    //....
}

Then you can have two implementations.

public class MathCalculator : IMathCalculator
{
    //....
    public int Factorial(int n)
    {
        //Implement factorial iteratively.
    }
    //....
}

public class RecursionEnabledMathCalculator : IMathCalculator
{
    //....
    public int Factorial(int n)
    {
        //Perform calculation recursively.
    }
    //....
}

This way, you can even skip the check of mode. While I agree with Christophe's note:

Exception: If the caller may know something that could allow a more appropriate way to do the calculation, and that the callee cannot find out by herself, then a strategy parameter is completely acceptable.

I would suggest that this first be dealt with as a design-smell. If some part of the code knows which specific implementation it needs, it better be well hidden, located deep within some library, where tiny details matter that much. Implementation details should only matter to other implementation details of the same context (at best). Implementation details of this kind (programming/mathematical optimizations) are very hard to debug or reason about and have very low maintainability in the case, when other maintainers take over.

By the way, your posted function does not work as expected when interpreted under the KISS principle. If I am interpreting it correctly, func(2, false) returns 3 and func(2, true) returns 0 (while func(-3, true) will simply cause a stack overflow exception). Bypassing the fact that any positive argument passed with true will return 0, based on this part of your question:

where the two calculations are not related but must be done to the same datum

If passing true or false with the same int n input returns different results, and this is intentional, then you better rethink your design because almost nobody reading only the function signature will understand anything, besides you or whoever has access to the source code and does not forget to check it out.

Also, to directly answer your question:

Under any circumstances would doing this be a better practice than simply having one function call two different functions that do the two jobs?

No, there are almost no such circumstances. If you mean which one is better, your posted example or this:

public static int func(int n, boolean mode) {
    if (!mode) return funcNonRecursive(n);
    else return funcRecursive(n);
}

private static int funcNonRecursive(int n) {
    return n + 1;
}

private static int funcRecursive(int n) {
    return n != 0 ? func(n - 1, mode) : 0;
}

then this is almost always better than your posted example, unless you care about the additional method call overhead, in which case you:

• Belong to a vast minority where you have to write extremely time-critical code.
• Probably shouldn't worry because modern compilers usually optimize this type of code (e.g. by inlining).

In any case, chances are very low so, if readability is your primary concern, this way is more human-friendly and maintainable.

Answer 2

The guiding principles here should be separation of concerns:

• The caller should invoke a function only with the parameters that are relevant for the result regardless how the calculation is performed;
• The called function should provide the result as best suit the needs, whether it's iterative, recursive, cached or a combination of any of these.

Exception: If the caller may know something that could allow a more appropriate way to do the calculation, and that the callee cannot find out by itself, then a strategy parameter is completely acceptable.

A function should also do one thing only and do it well. Coupling in one function two unrelated things should be avoided if the sole purpose of mode is to chose between the two.

Answer 3

Functions are one of our mechanisms for abstraction, and, one aspect of the quality of an abstraction is the usefulness to the consuming programmer, the caller.

Let's try to imagine the caller and since there are two different computations being done, what are the chances that the caller will use a variable for the boolean, rather than always passing a constant (true vs. false).

If it is true that always, the calling code knows which of the two computations they want, such that they are passing a boolean constant — then this approach is merely conflating two otherwise separate concepts, and thus increasing complexity for no value.

If, however, there is some reason to dynamically switch from one computation to the other, then it would seem to merit mixing two approaches together.  However, in that case, I would supply three versions: two without the boolean doing their individual jobs, and a third with boolean that invokes one of the other two.  Callers that know which they want would choose directly.

In any case, for one example, the IDE will be more helpful: it is harder to find all methods that pass specific parameters, rather than to find a more specialized method.  In general, maintenance and everything else is more difficult when we conflate concepts that don't need to be together.

You can see that the issue of whether the function is recursive vs. not is irrelevant to this analysis.  The issue to me is that there is no real merit to conflating two things that can otherwise be kept separate; no merit to the idea that these otherwise separate operations should be put together just because they both take and return an int (operate on the same data).

Answer 4

I'll answer this in Robert C. Martin's words from Clean Code:

Function Arguments

The ideal number of arguments for a function is zero (niladic). Next comes one (monadic), followed closely by two (dyadic). Three arguments (triadic) should be avoided where possible. More than three (polyadic) requires very special justification—and then shouldn’t be used anyway.

[...]

Flag Arguments

Flag arguments are ugly. Passing a boolean into a function is a truly terrible practice. It immediately complicates the signature of the method, loudly proclaiming that this function does more than one thing. It does one thing if the flag is true and another if the flag is false!

In Listing 3-7 we had no choice because the callers were already passing that flag in, and I wanted to limit the scope of refactoring to the function and below. Still, the method call render(true) is just plain confusing to a poor reader. Mousing over the call and seeing render(boolean isSuite) helps a little, but not that much. We should have split the function into two: renderForSuite() and renderForSingleTest().

So following this advice, your function should be split into two:

public static int func(int n) {
    return n + 1;
}

public static int funcRecursive(int n) {
    return n != 0 ? funcRecursive(n - 1) : 0;
}

Answer 5

Sure.

Your particular example doesn't make sense to me, but, there are plenty of operations that make sense to run recursively or not.

In particular, you might have a hierarchical structure that is expensive to traverse. When you search this structure or fetch statistics, there may be cases where the caller needs certainty (so you recurse) or approximation (guesstimate based on top level nodes or cached results).

And there are cases where the precise level of recursion should be configurable. Consider the --max-depth-types of arguments supported by Unix commands like du and tree, for example. If you're rendering a hierarchy to a user, it often makes sense to provide a mechanism to arbitrarily limit how much of the hierarchy you traverse and display.

Answer 6

The client is deciding two things simultaneously in your example:

1. What to do (add 1)
2. How to add 1 (recursively or not)

I cannot imagine a case when code like this is not:

1. Hard to understand and maintain and therefore more prone to bugs
2. Harder to unittest