[Ffmpeg-devel] [PATCH] video4linux2 input

[The Wanderer]

Michael Niedermayer wrote:

> Hi
> 
> On Wed, Feb 01, 2006 at 12:35:38PM +0100, Diego Biurrun wrote: [...]
> 
>>> (doxygen) comments are always good if looking at the
>>> function/loop/... for a minute isnt enough to understand it, and
>>> they are often bad if the code is trivial anyway (its like adding
>>> whitespace or random junk which makes reading harder, allthough
>>> it gives the impression to some people the code becomes more
>>> understandable, its like 5min to understand 1 line vs. 5min to
>>> understand 1 page of code but both do the same thing, later gives
>>> the impression of being easier to understand ...)
>> 
>> Please don't forget that not everybody has your coding skills.
>> Lesser coders might profit more from a few explanatory comments.
> 
> yes, but comments should be writen carefully, there are a few stupid
> things people frequently do
> 
> 1. choose bad name and fix it by a comment: for example:

I've been known to do that on occasion, albeit only after spending five
minutes doing nothing but staring at the screen trying without success
to think of an appropriate name. At some point you just want to start
coding and leave the rest to work itself out later; appropriate names
can be more obvious once the structure of the code is in place.

> this should obviously be an enum or #define

...apparently there are still major C features I don't know about;
although I've heard the term, I don't actually know what an enum is. I
suppose it's back to Google, or would be if I had the time to care right
now...

> 2. use a comment for an assert() not only cant that be checked
> automatically, its also often unclear if the comment is some optional
> debuging code or some always true statement

...if I understand your examples on this one (none of which contained an
assert() - ?), then yes, I agree.

> 3. simply unneeded comments which make reading more difficult:
> 
> h261.c:    // QCIF
> h261.c-    if (width == 176 && height == 144)
> h261.c-        return 0;
> h261.c:    // CIF
> h261.c-    else if (width == 352 && height == 288)
> h261.c-        return 1;
> h261.c:    // ERROR
> h261.c-    else
> h261.c-        return -1;

I would write that (IMO more legibly) as something like:

if (width == 176 && heignt == 144) {
     // QCIF
     return 0;
} else if (width == 352 && height == 288) {
     // CIF
     return 1;
} else {
     // ERROR
     return -1;
}

(although I would be likely to use block-style comments, just out of
habit). I do *not* think that these are "unneeded comments", since it is
not obvious to someone who is reading the code exactly what each test
corresponds to. It would be possible to avoid them by defining symbolic
names to be used as the return values, or to be used in the if() tests
themselves - but some indication of exactly what these tests are
checking is indeed appropriate, and brief comments are one easy way to
provide that.

(The above note is the main impetus for my sending this reply in the
first place; the rest of my post is mainly "because it's there" filler.)

> 4. using comments to document logical subparts of a function instead
> of spliting the function into several, look at any file i wrote for
> examples ;)

There are times when it can be difficult to assess whether or not to
split a function, though; also, although this could be seen as more like
what you put under case 3, one- or two-line chunks of code whose
function is not immediately obvious can sometimes be worth explaining
briefly. (I've got a few such in the code I've been writing recently,
having taken one of my longstanding frustrated projects off the back
burner and started writing tools to make it more feasible.)

> 5. writing comments in a language other then english

Or, more specifically, in a language other than one which most of the
people who will read it can be expected to fluently understand - which,
in practice, tends to work out to "a language other than English".

-- 
       The Wanderer

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

Secrecy is the beginning of tyranny.