[FFmpeg-devel] [PATCH 2/2] dshow: handle events in graph

[Don Moir]

----- Original Message ----- 
From: "Reimar Döffinger" <Reimar.Doeffinger at gmx.de>
To: "FFmpeg development discussions and patches" <ffmpeg-devel at ffmpeg.org>
Sent: Sunday, December 09, 2012 12:56 PM
Subject: Re: [FFmpeg-devel] [PATCH 2/2] dshow: handle events in graph


> On Sun, Dec 09, 2012 at 12:45:05PM -0500, Don Moir wrote:
>> Ok some people are trying to remove braces and some keep adding
>> them... I hope its not a brain freeze if you have to add them later
>> at some separate time :) For me personally it just adds to the code
>> clutter which is already prevalent.
>
> We prioritize reducing clutter in patches over minimal clutter
> in code.
> The result of that is to add {} whenever that does not require
> extra lines,

The following always has an extra line:

if (...) {
    single statement;
} // extra line

> since it has the potential of improving readability

>From an outsider looking in, readability is not one of ffmpegs strong points. I know one of the rules is to comment non-trival 
functions but thats rarely done for internal functions. Also naming of variables for AVFormatContext, AVStream varies and with terse 
naming. When looking thru the code I have to keep looking at what 's' is etc.

> of future patches significantly (when someone adds a new {
> you'd otherwise have to carefully check whether or not they
> also changed the if condition).

Should be a rare event for the most part. I really hate to see code like:

if (...) {
    single statement;
} else if (...) {
    single statement;
}

Thats messy. I prefer the following for readability.

if (...)
    single statement;
else if (...)
    single statement;

I also don't use K&R. I started out that way but one company way back forced us to change. We bitched a couple days and then went 
silient because we realized it was more readable to do:

if (...)
{
    single statement;
    single statement;
}

When things get more complex with nesting etc its easier to follow IMO. Note did not use IMHO: humble programmer or programmer with 
humble opinion does not compute :)

A minor side effect is its easy to comment out the test case which sometimes I do during development for testing.

// if (...)
{
    single statement;
    single statement;
}

The good thing is you have coding rules but probably not strict enough.