[MPlayer-cvslog] r28360 - in trunk: Makefile mencoder.c mpcommon.c mpcommon.h mplayer.c version.sh

[Aurelien Jacobs]

On Sun, 22 Feb 2009 11:39:34 +0100
Diego Biurrun <diego at biurrun.de> wrote:

> On Sat, Feb 21, 2009 at 10:49:14PM -0500, The Wanderer wrote:
> > Diego Biurrun wrote:
> > 
> > > On Sun, Jan 25, 2009 at 11:54:53PM +0100, Aurelien Jacobs wrote:
> >  >
> > >> diego wrote:
> > >> 
> > >> > Log:
> > >> > Factorize print_version().
> > >> > 
> > >> > [...]
> > >> > @@ -24,6 +28,47 @@ ass_track_t* ass_track = 0; // current t
> > >> >  
> > >> > +#if HAVE_MMX
> > >> > +    mp_msg(MSGT_CPLAYER,MSGL_V," MMX");
> > >> > +#endif
> > >> > +#if HAVE_MMX2
> > >> > +    mp_msg(MSGT_CPLAYER,MSGL_V," MMX2");
> > >> > +#endif
> > >> > +#if HAVE_3DNOW
> > >> > +    mp_msg(MSGT_CPLAYER,MSGL_V," 3DNow");
> > >> > +#endif
> > >> > +#if HAVE_3DNOWEX
> > >> > +    mp_msg(MSGT_CPLAYER,MSGL_V," 3DNowEx");
> > >> > +#endif
> > >> > +#if HAVE_SSE
> > >> > +    mp_msg(MSGT_CPLAYER,MSGL_V," SSE");
> > >> > +#endif
> > >> > +#if HAVE_SSE2
> > >> > +    mp_msg(MSGT_CPLAYER,MSGL_V," SSE2");
> > >> > +#endif
> > >> > +    mp_msg(MSGT_CPLAYER,MSGL_V,"\n");
> > >> > +#endif /* RUNTIME_CPUDETECT */
> > >> > +#endif /* ARCH_X86 */
> > >> > +}
> > >> 
> > >> Unrelated to this commit, but this could be written nicer, smaller,
> > >> more readable:
> > >> 
> > >> if (HAVE_MMX)    mp_msg(MSGT_CPLAYER,MSGL_V," MMX"  );
> > >> if (HAVE_MMX2)   mp_msg(MSGT_CPLAYER,MSGL_V," MMX2" );
> > >> if (HAVE_3DNOW)  mp_msg(MSGT_CPLAYER,MSGL_V," 3DNow");
> > > 
> > > Done, thanks for the hint.
> > 
> > As far as I can tell, it wasn't done, at least not as suggested.
> > 
> > As I read it, the important part of the suggested change was the
> > readability increase, which in turn derives almost entirely from moving
> > the mp_msg call onto the same line as the conditional and doing vertical
> > alignment as appropriate. In order to achieve that, it's necessary to
> > convert the #if X into if(X), but that change is not itself especially
> > important.
> > 
> > Commit 28375, which I think is the one corresponding to the change
> > you're talking about, does the #if -> if() conversion but does not
> > combine the lines. I don't think I see how this is an improvement.
> > Shouldn't the rest of the suggested change be made?
> 
> No, statements on the same line as the if are an abomination.

This is a stupid broad scholar statement. It's just the same as saying
"never ever use goto".
Most of the time, statements on the same line as the if makes the
code harder to read. But there are some situations where this is
just the opposit. And the code we are talking about is one of the
best example where statements on the same line as the if is much
more readable. Just write both versions, show them side be side,
and then come back telling me that your version is more readable...

Anyway, I don't care at all about this code, so mess it up as
much as you want.

Aurel