[MPlayer-cvslog] CVS: main/libmpcodecs pullup.c,1.20,1.21

[The Wanderer]

Reimar Döffinger wrote:

> Hi, On Sun, Feb 06, 2005 at 12:49:35PM -0500, The Wanderer wrote:
> 
>> D Richard Felker III wrote:
>> 
>>> On Sat, Feb 05, 2005 at 01:01:56AM +0100, Felix Buenemann wrote:

>>>> a few parentheses too much never hurt readability of code or
>>>> mathematial formulas aswell. If in doubt, use em :-)
>>>> 
>>>> It's a bit like:
>>>> 
>>>> if(x) for(;x<y;x++) if(x%2) do_some_thing(x);
>>>> 
>>>> versus:
>>>> 
>>>> if(x) {
>>>>  for(;x < y; x++) {
>>>>    if(x%2) {
>>>>      do_some_thing(x);
>>>>    }
>>>>  }
>>>> }
>>>> 
>>>> Both are syntactically correct, but the first version is surely
>>>>  more error-prone when changing code and less readable.
>>> 
>>> I disagree STRONGLY! The first version is much more readable to
>>> me, for two reasons:
>>> 
>>> 1. it doesn't clutter the surrounding code with lots of useless
>>>    lines and needlessly separate what comes before and after by
>>>    lots of distance.

(Condensing my apparent argument here: I don't necessarily agree with
either "useless" or "needless".)

>>> 2. it's a lot more internally readable.

(I disagree; I think that, at best, it's equally readable.)

>> Well, FWIW, I find the second option to be - while, yes, more
>> cluttered in some sense - also more readable; it's simply that when
>> everything is all on one line as it is in the first example, it's
>> noticeably harder (i.e. requires more concentration) to tell where
>> statements begin and end, and in the case of nested parentheses to
>> tell which close matches with which open. Breaking it off into
>> multiple lines allows the statements to be separated out visually,
>> which makes it considerably easier to tell at a glance what is part
>> of something else and what is separate. It does, as I said, mean
>> more clutter in one sense, but it's less dense and contains more
>> 'signposts' (an obscure reference if ever I've made one), so I
>> think it comes out at worst equal in the end.
> 
> I think you are mixing two things here: putting all in one line,
> which IMHO is bad because especially after a few changes it will make
> the line longer that 80 characters and also because it makes cvs diff
> much more difficult to understand.

Agreed.

> But adding unneeded braces is not any better, it takes up much too
> much spaces - given the fact that I usually can only see 25 to 50
> lines of code at once

This could be a considerable part of our difference; I habitually work
in a window large enough to display closer to 80 lines at a time.

> I find it _very_ annonying when more than 10 consist only of braces.
> Also, if you have so many nesting levels it's really hard to see
> which closing brace belongs to which opening one.

The latter point can be handled by making sure that each closing brace
is aligned (indented) to match its opening statement and/or opening
brace - which I thought was standard practice anyway; I make it a habit
to always set a sub-block off with braces, even in the case where the
block is only one line long (as in the do_some_thing(x) function call
above). It helps me to tell that the single line is a separate block,
and where that block begins and ends; it may be a little redundant with
the indentation, but I find that indentation alone is not always enough
to make the result readily parseable.

I could be persuaded to agree that the practice of putting the open
braces on separate lines is a bad one; I don't do that myself. I do,
however, think that it is/can be worthwhile to put the closing brace on
a separate line, although that may vary as a function of available
screen space.

...it's just occurred to me that this kind of discussion probably
doesn't belong on the CVS-log list, and quite possibly not on the
MPlayer lists at all...

> That's almost like making a sub-function consisting of only 2 lines -
> there are case where this is a good idea, but they are _very_ rare.

<shrug> Apparently I disagree. I'll admit that it's not often that I've
found occasion to use a function of two lines or fewer (not counting
simple callbacks), but the situation is not quite the same, and I find
the additional degree of demarcation provided by enclosing all code
blocks in braces to be worthwhile.

-- 
       The Wanderer

Warning: Simply because I argue an issue does not mean I agree with any
side of it.

A government exists to serve its citizens, not to control them.