RE: cvs commit: jakarta-turbine/src/java/org/apache/turbine Turbi ne.java

["Diethelm Guallar, Gonzalo" <Go...@sonda.com>]

> dlr         01/07/24 22:39:03

Hi Daniel!

>   Modified:    src/java/org/apache/turbine Turbine.java
>   Log:
>   Removed early return (having only one exit point from a method is
>   preferable).

[This is a minor point, and I don't mean to start a religious war;
 if anybody feels like ignoring this, just go ahead... 8-)
 Even more, the example I provide may not apply for this particular
 change in cvs.]

I beg to disagree. I find it very hard to read code structured like this:

  ret = null;
  preA();
  if (A)
  {
    preB();
    if (B)
    {
      preC();
      if (C)
      {
        ret = do();
      }
      else
      {
        ret = notC();
      }
    }
    else
    {
      ret = notB();
    }
  }
  else
  {
    ret = notA();
  }
  return ret;

and find this is a lot more readable:

  preA();
  if (! A)
  {
    return notA();
  }

  preB();
  if (! B)
  {
    return notB();
  }

  preC();
  if (! C)
  {
    return notC();
  }

  return do(); 

In my opinion, it is clearer and more maintainable; at least, it
is easier to add a new case for D...

Regards,


-- 
Gonzalo A. Diethelm
gonzalo.diethelm@sonda.com

---------------------------------------------------------------------
To unsubscribe, e-mail: turbine-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: turbine-dev-help@jakarta.apache.org

Re: cvs commit: jakarta-turbine/src/java/org/apache/turbine Turbi ne.java

[Daniel Rall <dl...@finemaltcoding.com>]

"Diethelm Guallar, Gonzalo" <Go...@sonda.com> writes:

> >   Log:
> >   Removed early return (having only one exit point from a method is
> >   preferable).
> 
> [This is a minor point, and I don't mean to start a religious war;
>  if anybody feels like ignoring this, just go ahead... 8-)
>  Even more, the example I provide may not apply for this particular
>  change in cvs.]
> 
> I beg to disagree.

[snipped example code]

> In my opinion, it is clearer and more maintainable; at least, it
> is easier to add a new case for D...

Hi Gonzalo!

I totally disagree.  ;)

I've seen a lot of code written in both styles--hell, I used to use
the early return myself.  However, while I was hacking C code for
Brian Fox (original author of bash), he showed me the error of my
ways.  What about the important cleanup operations that occur at the
bottom of the method (i.e. releasing a database connection)?  Now, the
first time the method is written, one takes such things into
consideration (being the originator of the code).  However, the next
time some maintainence programmer comes through, or some OSS hacker
submits a patch to the method (your code is entirely foreign to them,
and they are of variable skill), and they stick in an early return for
the case where an input validation check fails.  Your cleanup
operation never occurs.  Expensive resources are never free()'d.  As
the code ages, issues of this sort occur more and more often, and
problems with the code become more pronounced.  Imagine if the code
was C or C++, and the method was recursive?  You'd have a segfault or
run out of memory in no time.  Now, considering this in the context of
a Java program, where the GC fairy does a lot of our work for us, it
perhaps seems not so scary.  But regardless, it is a best practice
which helps to produce bullet-proof, maintainable code.

I value reability very, very highly, but this is a case I would say it
takes second place to maintainability.  And the easy workaround for
the deep nesting and branching you pointed out as hard to read is just
to add more methods, encapsulating branches of the tree.  As you apply
these techniques for but a little while, any unease at the absense of
early returns leaves you quickly, and you grow accustomed to writing
solid code that lasts forever.

Lastly, I equate early return statements with goto statements of the
worst bread.  Early return statements move the instruction pointer
across entire stack frames, instead of just incrementing it in the
controlled manner of refined local goto statements like if, else, else
if, do, while, and for.

IMNSHO, the ideal this:

ONE entry point.  ONE exit point.  NO non-local goto statements.

- Dan

p.s. I will not be posting more messages on this topic unless there
are specific questions to address.  =)

---------------------------------------------------------------------
To unsubscribe, e-mail: turbine-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: turbine-dev-help@jakarta.apache.org