3 ms·

I always go a step further, and use at most one "return" in a function. I find short-cut "return" statements frustrating for many reasons. For example, there'

I always go a step further, and use at most one "return" in a function. I find short-cut "return" statements frustrating for many reasons. For example, there's a risk that proper cleanup will not always happen, it is harder to see state and program flow when debugging, and it's no longer simple to make every-case changes by adding things to the end of the function. They're really just another type of "goto". The only "advantages" I can think of are saving a few keystrokes, and having slightly better performance by not using a variable for the return value. In my experience, neither of those things has ever mattered. In other words, I'd do this: <type> result = defaultValue; if (condition) { result = val1; } else { result = val2; } return result; With this approach, there is never anything after the "return", "else" or otherwise.

tptacek 17y ago
I write this way, religiously, and literally every teammate I've worked with has been frustrated by it. I agree with all your arguments (and I'll add another one, which is that it makes the code itself easier to instrument --- which is why I started doing it), but I've been convinced not to try to convince others of it.
Legion 17y ago
I tend to do this too, but I'm not convinced yet that I'm doing the right thing. I'd love to hear other opinions on this style.
cperciva 17y ago
I always go a step further, and use at most one "return" in a function. I opt for one return per exit type. For example: * If I have a success/failure function, I will have "return (0);" and "return (1);" statements. * If I have a pointer-or-fail function (e.g., an object constructor), I will have "return (r);" and "return (NULL);" statements. * If I have a true/false/fail function (e.g., check if a file exists; return 1 if it exists, return 0 if it doesn't exist, and return -1 if something failed such that we couldn't figure out if the file exists), I will have "return (0);", "return (1);", and "return (-1);" statements. In trivial functions, this tends to make my code somewhat more verbose than necessary; but I've found that sticking to this coding style dramatically reduces the odds of me making mistakes when I come back to code later and modify it, since different exit types usually require different cleanup code.
mullr 17y ago
Your cleanup argument holds true for C and its ilk, but not for anything else. In C you DO have to do manual cleanup, but in any language with exceptions you can (and should) do that in a finally block. Or in C++, via one of the many scope-aware smart pointer classes. In higher level languages, it is idiomatically correct to have many exit points from a function. As for being harder to debug... it depends. Again, if you MUST do manual resource deallocation, then I can see your case. Having duplicated cleanup code inside the control flow gets old fast. But if you've got exceptions, then such a rule leads very quickly to unnecessarily deep, convoluted control structures. I've flattened many such structures by a) factoring chunks out to separate functions and b) returning early when appropriate. It is very common for a function/method to either throw an exception or return a default value based on some parameter. In these cases, it's pound-you-in-the-face clear to have a list of "if(check) return whatever;" clauses at the top. If you do this before resource allocation, you don't even need to worry about cleanup. This feels like a much "purer" way to write a function to me, almost like applying pattern matching before falling back to imperative mode.
- makecheck 17y ago
  That's true, and I do use that approach ("RAII") in languages like C++. It definitely makes cleanup less of a problem. It also means that setup should be done no earlier than necessary, declaring variables very close to their first use (otherwise, a setup/cleanup cost may be paid for no reason). Exceptions act like early returns themselves, but I will use them for truly exceptional cases (only). In other words, if a function can return 3 values, I consider those the "normal" cases that should weave through to the end of the function; if the function runs out of memory, I think that's an exceptional case that can return immediately.
antonovka 17y ago
Yuueurrgh. Without early return, you wind up with multiple-nested if statements that make the code absolutely unreadable. Early return is almost always used as the error path -- if you need to perform cleanup, then instead of returning, use goto to jump to a single end-of-function cleanup handler. There's nothing wrong with using goto for this purpose, and it means I can actually read -- at a glance -- code that requires more than a few preconditions at the start of the function.
- makecheck 17y ago
  Functions that have become too complicated should look ugly, so that they're likely to be split into simpler parts (for true readability). When making that split, it is also helpful to have relevant cleanup code nearby: odds are everything is already in the inner block, instead of having to remove extra code at the end of the function.
  - antonovka 17y ago
    That's the usual hand-waving argument (if you need this you're doing it wrong), but the fact is that even something with three simple preconditions is quickly rendered unreadable through three nested levels of if statements.
- anamax 17y ago
  > if you need to perform cleanup, then instead of returning, use goto to jump to a single end-of-function cleanup handler. That assumes that all of the cleanup is the same and that all of the relevant information that is available at the subroutine's top level.

tptacek 17y ago

I write this way, religiously, and literally every teammate I've worked with has been frustrated by it. I agree with all your arguments (and I'll add another one, which is that it makes the code itself easier to instrument --- which is why I started doing it), but I've been convinced not to try to convince others of it.

Legion 17y ago

I tend to do this too, but I'm not convinced yet that I'm doing the right thing. I'd love to hear other opinions on this style.

cperciva 17y ago

I always go a step further, and use at most one "return" in a function. I opt for one return per exit type. For example: * If I have a success/failure function, I will have "return (0);" and "return (1);" statements. * If I have a pointer-or-fail function (e.g., an object constructor), I will have "return (r);" and "return (NULL);" statements. * If I have a true/false/fail function (e.g., check if a file exists; return 1 if it exists, return 0 if it doesn't exist, and return -1 if something failed such that we couldn't figure out if the file exists), I will have "return (0);", "return (1);", and "return (-1);" statements. In trivial functions, this tends to make my code somewhat more verbose than necessary; but I've found that sticking to this coding style dramatically reduces the odds of me making mistakes when I come back to code later and modify it, since different exit types usually require different cleanup code.

mullr 17y ago

Your cleanup argument holds true for C and its ilk, but not for anything else. In C you DO have to do manual cleanup, but in any language with exceptions you can (and should) do that in a finally block. Or in C++, via one of the many scope-aware smart pointer classes. In higher level languages, it is idiomatically correct to have many exit points from a function. As for being harder to debug... it depends. Again, if you MUST do manual resource deallocation, then I can see your case. Having duplicated cleanup code inside the control flow gets old fast. But if you've got exceptions, then such a rule leads very quickly to unnecessarily deep, convoluted control structures. I've flattened many such structures by a) factoring chunks out to separate functions and b) returning early when appropriate. It is very common for a function/method to either throw an exception or return a default value based on some parameter. In these cases, it's pound-you-in-the-face clear to have a list of "if(check) return whatever;" clauses at the top. If you do this before resource allocation, you don't even need to worry about cleanup. This feels like a much "purer" way to write a function to me, almost like applying pattern matching before falling back to imperative mode.

makecheck 17y ago
That's true, and I do use that approach ("RAII") in languages like C++. It definitely makes cleanup less of a problem. It also means that setup should be done no earlier than necessary, declaring variables very close to their first use (otherwise, a setup/cleanup cost may be paid for no reason). Exceptions act like early returns themselves, but I will use them for truly exceptional cases (only). In other words, if a function can return 3 values, I consider those the "normal" cases that should weave through to the end of the function; if the function runs out of memory, I think that's an exceptional case that can return immediately.

antonovka 17y ago

Yuueurrgh. Without early return, you wind up with multiple-nested if statements that make the code absolutely unreadable. Early return is almost always used as the error path -- if you need to perform cleanup, then instead of returning, use goto to jump to a single end-of-function cleanup handler. There's nothing wrong with using goto for this purpose, and it means I can actually read -- at a glance -- code that requires more than a few preconditions at the start of the function.

makecheck 17y ago
Functions that have become too complicated should look ugly, so that they're likely to be split into simpler parts (for true readability). When making that split, it is also helpful to have relevant cleanup code nearby: odds are everything is already in the inner block, instead of having to remove extra code at the end of the function.
- antonovka 17y ago
  That's the usual hand-waving argument (if you need this you're doing it wrong), but the fact is that even something with three simple preconditions is quickly rendered unreadable through three nested levels of if statements.
anamax 17y ago
> if you need to perform cleanup, then instead of returning, use goto to jump to a single end-of-function cleanup handler. That assumes that all of the cleanup is the same and that all of the relevant information that is available at the subroutine's top level.