[MPlayer-dev-eng] [PATCH] libbs2b: new api

Andrew Savchenko bircoph at gmail.com
Wed Apr 22 17:10:29 CEST 2009


Hi,

On Tuesday 21 April 2009, Uoti Urpala wrote:
[...]
> > > +            else
> > > +            if (!strcmp(s->profile,"cmoy"))
> > >
> > > "else if" should be on one line.
> >
> > No, no, no. The idea is to keep all if statements at the same
> > alignment level, otherwise it will be mess to read. Currently
> > they have nice and easy-to-understand-the-code indentation.
> > Statements are logically equal and there is no reason to
> > accent one of them. You may consider this as swith statement
> > for strings.
> >
> > Yes, I know this is not strict K&R indentation but sometimes
> > logics should override common rules.
>
> "switch statement with if" is a common case, not in any way an
> exception. Your version is not objectively better. You can make
> up arguments at least as good as your "logically equal" against
> it if you want. I find it uglier in practice too.

All indentation beauty or ugliness is highly subjective by 
definition. This is normal because different people have different 
tastes.

> Inventing your own code formatting rules for common cases is
> simply a bad idea.

I can't agree with this statement. Thanks God there is no such 
thing as ISO standard or RFC recommendations for coding style, so 
everyone is free to use any style. There are no standards and will 
never be.

Paragraph 6 of tech/svn-howto.txt states:
Every developer has his own indentation style, you should not 
change it. Of course if you (re)write something, you can use your 
own style... (Many projects force a given indentation style - we 
don't.)

This is definitely outdated. Currently indentation style is forced 
in MPlayer to strictly conform K&R style. That's all. I'll follow 
K&R if all of you are so insist on it, but this paragraph must be 
changed to explicitly indicate that K&R style is mandatory and 
code formatted in any other way will be rejected.

> > > +               
> > > bs2b_set_level(s->filter,BS2B_JMEIER_CLEVEL);
> > >
> > > Space missing after ",".
> >
> > Ok.
>
> Still has at least this:
> +            bs2b_set_level_fcut(s->filter,s->fcut);
> +        if (s->feed)
> +            bs2b_set_level_feed(s->filter,s->feed);

I added spaces between function argument troughover all code.

-- 
Best regards,
Andrew
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20090422/a7357b6a/attachment.pgp>


More information about the MPlayer-dev-eng mailing list