[Ffmpeg-devel] [PATCH] video4linux2 input (original) (raw)

The Wanderer inverseparadox
Wed Feb 1 21:49:00 CET 2006

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.

More information about the ffmpeg-devel mailing list