Re: Control Flow Coding Style

   I agree in principle but disagree in detail.

--- In [email protected], "Avdi Grimm" <avdi@...> wrote:
>
> On Mon, Apr 28, 2008 at 4:55 AM, Michael Hunger <pragmatic@...> wrote:
> 1. Short-circuit clauses at the beginning of methods:
> 
   I agree with the example here.
> ...  
>
> I like to keep these in a separate clause at the beginning of the
> method, though; once the primary logic of the method begins, no more
> short-circuit returns allowed.
>
   It's good to try to do this; it's not always the clearest way (see
below).

> ...
> 2. Methods where there is a consistent and obvious pattern of returns:
> 
>   def bar(arg)
>     arg = transform1(arg)
>     return arg if arg.ready?
>     arg = arg transform2(arg)
>     return if arg.ready?
>     arg = transform3(arg)
>     return arg
>   end
> 
   I try to avoid these where possible.  There's unnecessary
duplication here that is error prone.  Note the missing/misplaced
"arg" in the return after transform2.  I know it's pseudocode and
wasn't run through a compiler, and unit tests would probably catch a
returned nil, but in principle it's dangerous.

   I'd probably write this as 

  def bar(arg)
    arg = transform1(arg)
    if arg.notReady?    arg = transform2(arg)
    if arg.notReady?    arg = transform3(arg)

    return arg
  end

depending on language (in C/C++ I'd probably use braces even for
single statements, in Perl I might use a trailing [statement modifier]
if).

   This is partly my experience with early C++ (and generated
assembly).  The cfront compiler would not inline the original
multi-return form but would inline the rewrite.  The reason is
obsolete, but the flow complexity that caused it isn't.  I can live
with the redundant checks if they're not too expensive.

   I might also have a statement

    if arg.notReady?    arg = nil

just before the return (a counterexample to having all error
checks/returns at the beginning).

> What I *don't* like to see is a method that has one obvious return at
> the end, and then a second special-case return buried a couple layers
> deep in conditionals.  Those are the methods that lead to
> head-scratching debugging sessions because you haven't noticed that
> the method is exiting early.
> 
   Definitely agree.

> >  2) each if must have an accompanying else branch
> 
> The longer I write code, the more I think this is a good idea.  I
> pretty habitually put in 'else raise "Should never get here"' clauses
> in my code.  Yeah, it's a little extra clutter; but the time savings
> in catching bad assumptions early is worth it IMO.
> 
   Don't agree here.  My rewrite is my counterexample.  Often an if is
there to ensure an assertion is true at some point in the program
rather than to perform actions based on conditions.
> ...
>
       Neil