[MPlayer-dev-eng] [PATCH] Optional parameter for switch_audio

Rich Felker dalias at aerifal.cx
Fri Apr 22 01:44:10 CEST 2005


On Thu, Apr 21, 2005 at 05:50:14PM -0400, The Wanderer wrote:
> >+    case MP_CMD_SWITCH_AUDIO : {
> >+        int v = demuxer_switch_audio(demuxer, cmd->args[0].v.i);
> >+        if (identify)
> >+          mp_msg(MSGT_GLOBAL, MSGL_INFO, "ID_AUDIO_TRACK=%d\n", v);
> >+    } break;
> 
> Why create the variable "v" at all, instead of simply calling
> demuxer_switch_audio directly into the mp_msg call? Unless my

IMO using the temp var is better style. It's more readable... and
consider what would happen if someone commented out the message line
trying to just prevent the message from being displayed, or if mp_msg
were defined as a macro where the args might be evaluated more than
once or not at all.

IMO it's bad style in general to call functions with heavy side
effects as an argument to a printf-type function. You're hiding code
with real side effects inside an apparently-useless function call.

Rich





More information about the MPlayer-dev-eng mailing list