Re: Which code style do you prefer the most?

On 26.02.2025 15:58, David Brown wrote:

On 26/02/2025 14:06, Janis Papanagnou wrote:

On 26.02.2025 12:53, Ar Rakin wrote:

Janis Papanagnou <janis_papanagnou+ng@hotmail.com> writes:

>
Re "goofy style"; I've seen someone preferring
>
   while (q(a,b))
       {
       a=b;
       f(x);
       if (c>d)
           {
           g(y);
           }
       }
>
To each his own.

>
That looks like a nightmare for the code reviewers.

 
I find that style grating.  But I don't think I'd call it a "nightmare"
- I've seen far worse.
 
But from all my education and training in coding, mathematics,
documentation, writing, and typography, I am a solid believer in one
rule - the most important feature of any written information is the
spacing.  If I were tasked with making that code clear, the first step
would be to add a few spaces - "a = b;", "if (c > d)" - that would be
higher priority than using a more "normal" brace style such as the "One
True Brace Style".

Just note that this was _ad hoc_ code only to illustrate the braces
placement, not the spacing. (The guy I was talking about writes in
fact readable(!) code, i.e. with spacing, sensible naming, comments,
etc. etc.) The only really strange thing in his code are the braces
placement that I've not seen anywhere before. (And I've seen a lot
code, most of it complying more or less to prevalent conventions.)

 

>
I cannot tell where that comes from; the person who uses it is an
experienced Perl programmer - may that be some convention in that
specific language context? (I can't tell.)
>
WRT code reviews; in the past I had horrendous code that I needed
to reformat using a code-beautifier before I could review it. But
those were extreme (and only rare) cases. (And above yet isn't as
bad as those other artworks I had.)
>

 
I think it is more common in such cases to reject the code from the
review, and insist that the author re-format it appropriately.  The
reviewer's job is to /review/ the code, not fix it or clean it up.

Yes, but there are situations when you have to review code where
the author isn't available any more, as in the as said rare cases
I reported about.

In another somewhat different case we had to rewrite a piece of
code to add new functionality but keep (unspecified!) existing
functionality. That was not exactly a "review" but actually an
analysis, close to a black-box analysis. Reformatting was the
least effort compared to the behavioral analysis and stepwise
refactoring, using Extreme Programming methods, Pair Programming
and ad hoc introduction of regression tests. *That* was a nightmare!

Yes, the simple case is to just reject code. Or to let automated
tools run across the code to reject it in the first place without
any human interaction.

 
One thing that is important for code reviewers, and any kind of
comparisons between versions (such as for source code management
systems), is to try to reduce the number of lines that are changed
unnecessarily, and to try to make those changed lines clear.

What are you trying to suggest here? (It looks like I've never
had a case where it was important to compare source code with
previous versions just to remove "unnecessary" lines. - This
sounds really strange to me, but you may want to explain it.)

Janis

[...]