Sigbjřrn Skjćret wrote:
>
> >> and parsing gets abit more messy...
> >How ?
> [...]
>
> This is how it looks now, better than using va_arg() at each case imho...
Why is it better ?
You wasted an additional variable and gained ( maybe ) some code space
by not doing a function call ( or maybe it is a macro , probably
platform dependant ) every time.
By writing almost the same code 3 times you increase the likehood of
bugs 3 times :-)
Alltough most of the code would be repeated anyway.
My "more clear" version of the day :
( after the code are some more replyes to the original message )
int lame_set_parameters(lama_global_flags *gfp,...)
{
va_list args;
LAME_TAG_TYPE current_tag;
va_start(args);
current_tag = va_arg(args,LAME_TAG_TYPE);
while(current_tag != LAME_TAG_END )
{
#define CASE(TAG,elem,type) case LAME_ ## TAG : gfp -> elem =
va_arg(args, type); break;
switch(current_tag) {
CASE( SET_SAMPLING_FREQ , samp_freq , int )
CASE( SET_NR_CHAN , nr_channels , int )
CASE( FILTER_WIDTH , filter_width , float )
CASE( SOME_COMMENT , some_comment , char* )
/* some other integer, string or float or whatever parameters
..... */
#undef CASE
default : return RET_TAG_ERROR_CODE; /* also print a warning ?
probably not if this is a library.
default : return -current_tag; so the app knows which tag is
problematic
or maybe return the position of the
bad tag ? */
}
current_tag = va_arg(args,LAME_TAG_TYPE);
}
va_end(args);
}
Here is the old version for reference :
( there is a bug already !
it should be lame_struct -> xxx and not
lame_struct.xxx
)
lame_set_parameters(lama_global_flags *lame_struct,...)
{
va_list args;
LAME_TAG_TYPE current_tag;
va_start(args);
current_tag = va_arg(args,LAME_TAG_TYPE);
while(current_tag != LAME_TAG_END )
{
switch(current_tag) {
case LAME_SET_SAMPLING_FREQ : lame_struct.samp_freq =
va_arg(args,int); break;
case LAME_SET_NR_CHAN : lame_struct.nr_channels =
va_arg(args,int); break;
case LAME_FILTER_WIDTH : lame_struct.filter_width =
va_arg(args,float); break;
/* some other integer, string or float or whatever parameters */
....
}
current_tag = va_arg(args,LAME_TAG_TYPE);
}
va_end(args);
}
> void lame_set_int(lame_global_flags *gfp, lame_param_tag tag1, ...)
> {
> va_list ap;
> int param; /* (varargs promotes integrals to int) */
>
> va_start(ap, tag1);
/* in my man page va_start has only one argument (ap) ... */
> while (tag1 != TAG_END)
> {
> param = va_arg(ap, int);
>
> switch (tag1)
> {
> case LAMEtag1:
> gfp->test1 = param;
> break;
>
> case LAMEtag2:
> gfp->test2 = param;
> break;
>
> default:
> break;
> }
>
> tag1 = va_arg(ap, lame_param_tag);
> }
> va_end(ap);
> }
>
> I can still implement a way to have the tag also contain the type of the
> parameter, but since the general consensus seemed to be that this would be
> too complicated I went for this approach... ;)
But your existing tag "contain" the type , at least as much as they
would
in my method.
#ifdef YOU
#define SOME_TAG some_int_value
#else
#define SOME_TAG some_int_value
#end
Unless I use
SOME_TAG__WARNING_THIS_IS_FLOAT_AND_NOT_INT_OR_ANYTHING_ELSE,
but that is not the point , since it would work both with your and my
code.
I'm in hurry, forgive any mistakes :-)
> - CISC
>
> --
> MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
--
David Balazic
--------------
"Be excellent to each other." - Bill & Ted
- - - - - - - - - - - - - - - - - - - - - -
--
MP3 ENCODER mailing list ( http://geek.rcc.se/mp3encoder/ )
