Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [jdt-dev] A matter of style: loops vs. streams and - gasp - braces

Andrey,

I sure know about all the bad things that _can_ happen, if badly formatted code is misunderstood. No arguing about that. Well: also braces can be wrongly indented, which causes the same kind of havoc ...

It's just that in my universe this threat is so tiny (I'd have to turn my brains inside out to remember even a single instance) that the effort invested against it feels completely out of proportion. And if you have the problem I'd first think about investing in sane indentation.

Obviously, your millage differs from mine.

OK, let's call a truce then ...
Stephan

PS: Maybe I should propose to Oracle to introduce short forms of for and if for fluent imperative programming :)

On 28.09.19 17:06, Andrey Loskutov wrote:
Hi Stephan,

I was personally beaten few times (not in my code) by missing braces,
therefore I always prefer this:

      for (X x : coll) {
          if (isGood(x)) {
               process(x);
          }
      }

over this:

       for (X x : coll)
           if (isGood(x))
               process(x);

or this

for (X x: coll) if (isGood(x)) process(x);

or this

       for (X parent : coll) {
           if (isGood(parent))
               for (Y child : parent.getChildren())
                    process(child);
       }


I was beaten in two cases: reading that braceless code with wrong
indents and fixing bad merges / bad refactorings.

If you debug such code (not written by you), you tend to read this wrong:

       for (X x : coll)
           if (isGood(x))
               process(x);
               andProcess(y);

and this:

for (X x: coll) if (isGood(x)) process(x); andProcess(y);

and also this:

       for (X parent : coll) {
           if (isGood(parent))
               for (Y child : parent.getChildren())
                    process(child);
                    andProcess(parent);
       }

and finally this masterpiece:

       for (X parent : coll) {
           if (isGood(parent))
               process(parent);
               for (Y child : parent.getChildren())
                    process(child);
                    andProcess(parent);
       }

This is because our brain "automatically" prefers indentation over
"invisible" blocks, it sorts the bad indented lines to previous.

If we would use braces, such examples above would be simply not possible.

How this happen: easy. Either people use tabs + spaces mix, and use
different tabs settings, or you had two branches, a merge conflict with
lot of merges and you end with the code that perfectly compiles but does
not what you want.

Therefore I not only recommend to use braces in all cases but also set
this as the save action in our Eclipse settings.

Regarding lambdas: they are there to allow "fluent" code, so if one
needs to use braces in lambdas it a sign that this code should be
refactored to something else.

On 28.09.2019 15:20, Stephan Herrmann wrote:
Hi team,

(The following debate is as old as the invention of braces, but Java 8
gives a new twist to it, so please bear with me for minute as I turn
your world upside down).


In a current gerrit I found the following comment:

 > Also add missing braces on loops as necessary.

I doubt that any of those braces where "necessary", otherwise those
would have been bugs, no?


I'd like to actively promote a braceless style for simple loops and ifs
for the following reason:


Since Java 8 and the introduction of streams we have two main options
for writing many things involving iteration over a collection. As a
trivial first-level criterion to choose among the two, everybody uses
"conciseness" of the code.

Take this pattern:
    for (X x : coll) {
        if (isGood(x)) {
             process(x);
        }
    }
vs.
    coll.stream().filter(x -> isGood(x)).forEach(x -> process(x));

Everybody can "clearly see" that the latter variant is "better".


NO, it is not. We simply cheat by choosing a more condensed layout.
In terms of conciseness, this is the winner:
    for (X x: coll) if (isGood(x)) process(x);

Do you see, how the initial example compared apples and oranges??


The stream() variant even has its own drawback: it will severely
backfire should you ever need to debug that code section.


So my proposal is: Let's for a compromise explicitly admit this style:
     for (X x : coll)
         if (isGood(x))
             process(x);

IMHO this is the sweet spot to balance conciseness, readability and
debuggability.

For slightly more involved cases I made a habit of using braces for the
outer-most structure, thus delineating a meaningful chunk of code, s.t.
like
     for (X parent : coll) {
         if (isGood(parent))
             for (Y child : parent.getChildren())
                  process(child);
     }

Remember, that we provide quick assists for converting to block, should
we need to add more statements to some body.


If you don't like any of this, we should for fairness also forbid
braceless lambdas and enforce line breaks around the lambda body:
    coll.stream().filter(x -> {
        return isGood(x);
    }).forEach(x -> {
        process(x);
    });
This style is suitable for comparison with the old, imperative style
(but ugly, right?)


No, I'm not against lambdas (just a bit wary about debuggability), but
visual comparison between options should be fair :)


Put differently: a loop structure that can be written without braces has
the same mental clarity as a well structure stream-pipeline. Let's
please not penalize such style by unnecessary verbosity.

best,
Stephan
_______________________________________________
jdt-dev mailing list
jdt-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe
from this list, visit
https://www.eclipse.org/mailman/listinfo/jdt-dev

--
Kind regards,
Andrey Loskutov

https://www.eclipse.org/user/aloskutov
---------------------------------------------
Спасение утопающих - дело рук самих утопающих
---------------------------------------------
_______________________________________________
jdt-dev mailing list
jdt-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://www.eclipse.org/mailman/listinfo/jdt-dev



Back to the top