4

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?

  • 43
    "where the two calculations are not related" - in that case, create separate functions, give them meaningful names, and call them from wherever in the code you can make the decision. Otherwise it's just confusing. If you are concerned about performance, that is almost certainly not going to be an issue in practice, and clarity and readability is going to be more important. The best way to tell is to measure, though - create both versions and test to see if the difference is significant in the context of what you are trying to do. – Filip Milovanović Nov 2 '19 at 16:05
  • 1
    Agreed with Filip. Make them two separate functions. – Robert Harvey Nov 2 '19 at 19:41
  • 1
    This isn;'t recursive at all, if coded properly. The second option is just a time-wasting way to compute else return 0; which relies on undefined behaviour if n < 0. – alephzero Nov 3 '19 at 3:35
  • 7
    The sample code is misleading in the sense that the simple arithmetic (dummy code) has distracted too many answerers. To attract quality answers, the sample code should specify the qualitative attributes that best describes the situation (e.g. "recursively traverse a tree-like data structure", "scans a list of items", "performs a depth-first search or breadth-first search", "repeatedly apply map-reduce on a list of items (each output becomes the input to the next run) until the list no longer changes", etc.) – rwong Nov 3 '19 at 8:24
  • 2
    Is this a purely theoretical question with no relation to anything practical, or are you in an actual situation where you're considering doing something like this? If the latter, can you please describe the situation a little bit? It's hard to say whether this practice is a good idea "under any circumstances," but it's a lot easier to say whether or not it's a good idea under your circumstances. – Tanner Swett Nov 3 '19 at 12:19
4

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.

| improve this answer | |
19

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.

| improve this answer | |
  • 1
    Is callee a human? Did you mean itself instead of herself? – Ruslan Nov 3 '19 at 9:22
  • @Ruslan Thanks for the improvement suggestion! edited! – Christophe Nov 3 '19 at 9:26
  • If the caller "knows something", shouldn't the responsibility for not only choosing but implementing the choice of strategy fall upon it? i.e. externalize-the-ternary into the caller? Not clear on when or why it would be a good idea to separate the choice, from the decision point itself - seems that by deferring it (separating production of the condition flag from use of the condition flag) you're mainly just reducing some clarity and cluttering the API with someone else's responsibilities, with all the associated risks of that. – Leushenko Nov 3 '19 at 13:53
  • 1
    @Leushenko: (1) It still makes sense for a library to provide the implementations of algorithms - the caller may know more about "which" to use, but the library may know more about "how", in particular on issues of code optimization and correctness of implementation. (2) It is still recommended to provide these implementations as separate functions. (3) For library design, for large libraries that have "too many verbs" (too many API entry points), it makes sense to consolidate API entry points, organized by fewer "main verbs" and passing in additional strategy parameters. Example: OpenCV. – rwong Nov 3 '19 at 21:46
5

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

| improve this answer | |
  • In this particular case, recursion doesn't even make much sense. There will only ever be at most one recursive call, and the result will always be 0 in any case, so why even do the recursive call at all? – Jörg W Mittag Nov 2 '19 at 15:45
  • @JörgWMittag If you pass in 10, it will recursively call itself with 9, then 8, and so on until 0. Good luck if you pass in -1! – CJ Dennis Nov 3 '19 at 2:47
  • @CJDennis: You are referring to the completely different code that is in the question now. Not being in the possession of a time machine, I am referring to the code that was in the question at the time when I wrote the comment. – Jörg W Mittag Nov 3 '19 at 9:34
  • @JörgWMittag OK. It doesn't really matter since the code was just an example for using a flag variable. – CJ Dennis Nov 3 '19 at 9:39
1

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;
}
| improve this answer | |
0

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.

| improve this answer | |
  • 1
    I would likely only have this kind of parameter if the function needed to determine when to stop recursion. In general, i'd argue that if the logic is truly different based on the boolean, then that really should be a different function. You basically have two code paths, and you choose which one based on the argument. That's a switch statement disguised as a function call. ;] – Jon E Nov 2 '19 at 18:16
  • @JonE Maybe. Maybe it's more clear if you're not thinking of it as a mode or do_not_recurse flag ... unless the function explicitly warrants it, which is conceivable, isn't it? ... I want to search(), index(), or delete() a collection of documents with links to other documents. Makes sense to have a recurse flag and/or a recursion_limit flag. Like how you might configure SQL tables to CASCADE or not ... – svidgen Nov 2 '19 at 18:22
  • But, in the case where I'm running stats against a tree of data, maybe my recurse/no-recurse flag isn't named recurse at all. Maybe it's called force_recalculation or min_freshness_seconds something. – svidgen Nov 2 '19 at 18:24
  • 1
    Sure. Just not this way. – Robert Harvey Nov 2 '19 at 19:41
0

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
| improve this answer | |

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.