Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes:
>> I thought we made this into
>>
>> if verbose
>> return run_command
>> else
>> return run_command_silently
>>
>> to help readers in the previous round already.
>
> FWIW we had quite a couple of reviews in the recent years which pointed
> out "unnecessary else" after returning or die()ing. Maybe we should make
> up our minds, and set a consistent rule to follow.
FWIW the pattern you are referring to is
do something;
if (error detected) {
return error(...);
} else {
perform
remaining
actions
the function needs
to complete
its task;
}
and those reviewers (including me) are absolutely right that such an
"else" is not just unnecessary but actively harms readability of the
flow of logic.
I am also absolutely right when I say what is quoted at the top
makes 100% more sense than
if (verbose)
return run_command();
return run_command_silently();
as these two are about doing the same thing a bit differently.
The way to think about the latter is that we won't have this "if
(verbose)" if there were a variant of run_command() that took a set
of option bits among which is a verbose bit, but instead would have
a single call to that function that returns.. So it's not like "in
an exceptional case, return after calling this function; otherwise
keep going, which happens to only return after calling another". It
is more like "here we need to return after spawning a command, but
depending on this bit, we spawn the command using a different
function".
Good programers recognize the difference between these two patterns
without being told, and mentors should teach and GSoC student should
learn that an overly simplified rule like "when 'if' block ends with
return, do not write 'else'" is harmful.
