[GRASS-dev] [GRASS-SVN] r60703 - in grass/trunk: display/d.vect general/g.gisenv gui/wxpython/animation lib/python/temporal raster/r.colors raster/r.external raster/r.in.bin raster/r.mapcalc raster/r.neighbors raster/r.out.bin raster/r.quant raster/r.resamp.filter raster/r.series raster/r.series.accumulate raster/r.series.interp raster/r.stats.quantile vector/v.colors vector/v.external.out

[Huidae Cho]

Maybe, we can use variadic macros in C99 to remove the first name argument.

#define G_option_exclusive(...) G__option_exclusive(NULL, __VA_ARGS__)


On Mon, Jun 9, 2014 at 10:54 AM, Huidae Cho <grass4u at gmail.com> wrote:

> I somehow agree with you that my interface can be cryptic and requires
> some interpretation because information is spread out. I think we can
> define something like:
>
>         // at most one option from a set
>         void G_option_exclusive(const char *name, ...);
>
>
>         // at least one option from a set
>         void G_option_required(const char *name, ...);
>
>
>         // if the first option is present, at least one of the other
>         //  options must also be present
>         void G_option_requires(const char *name, ...);
>
> Here, actually, name is not needed, but is there only for stdarg. And we
> use these functions:
>
> G_option_exclusive("format", &opt1, &opt2, &flag1, NULL); // No opt1,
> opt2, and flag1 at all OR only one of them.
> G_option_required("required", &opt1, &opt2, &flag1, NULL); // At least one
> of opt1, opt2, and flag1 must be given.
> G_option_requires("group1", &opt1, &opt2, NULL); // opt1 and opt2 must be
> used together.
>
> In each function, we can use the state variable in parser.c
> (st->first_option and st->first_flag) to find defined options and flags:
>
> opt = &st->first_option;
> while(opt){
>   struct Option *x;
>   va_start(ap, name);
>   while((x = va_arg(ap, struct Option *))){
>     if (x == opt) {
>       ...
>       break;
>     }
>   }
>   va_end(ap);
>   opt = opt->next_opt;
> }
>
> flag = &st->first_flag;
> while(flag){
>   similar code here...
> }
>
> On Mon, Jun 9, 2014 at 5:34 AM, Glynn Clements <glynn at gclements.plus.com>
> wrote:
>
>>
>> Huidae Cho wrote:
>>
>> > My implementation of the "exclusive" member, which you reverted, can
>> handle
>> > all these cases, I think. But since you reverted it, I think you didn't
>> > agree with my interface or implementation?
>>
>> Not really.
>>
>> [I also wanted to be able to discuss this starting from a blank slate,
>> without the temptation to take any specific approach as a "reference
>> point". But that alone wouldn't have been sufficient for reversion.]
>>
>> Mostly, I find the interface to be cryptic. Information which should
>> (IMHO) be in one place is spread out across the individual options.
>>
>> In order to understand the relationships, anyone reading the code
>> needs to examine all of the options, and collate them into groups
>> based upon matching group numbers. IOW, mentally replicate the
>> (non-trivial) algorithms which form the bulk of r60713. The fact that
>> this was deemed to require several examples (with comments) tends to
>> suggest that it isn't particularly intuitive.
>>
>> Apart from that, declaring "rules" would make the transition more
>> straightforward, as it mirrors the structure of the existing code
>> (i.e. the "if (expression) error();" statements).
>>
>> By itself, that wouldn't be sufficient reason to prefer a rule-based
>> approach if it is considered to be inferior on its own merits. But
>> given the extent of the changes, it might be enough to tip the balance
>> in the event that the choice between different approaches amounted to
>> a toss-up.
>>
>> > IMO, passing option/flag names to G_option_required() has its own
>> > disadvantage because changing option/flag names takes more effort. If
>> you
>> > have valid reasoning behind adding functions rather than adding a
>> member,
>> > just like I did, I would prefer to pass *Option and *Flag pointers to
>> those
>> > functions. But I guess it can be a little tricky to pass two different
>> > types of pointers.
>>
>> Indeed. I originally considered passing the struct pointers before
>> realising that approach wouldn't allow both flags and options to be
>> used interchangeably. At least, not without changing the structures to
>> carry some kind of type signature, although that wouldn't be
>> particularly difficult, given that both structures have to be created
>> via the appropriate functions.
>>
>> This issue can be mitigated somewhat by using opt->name in the call
>> rather than a string literal[1]. We'd still need to use a literal for
>> flags, though.
>>
>> [1] If we do that, modules which use "opt1", "opt2", etc for the
>> variable names should be changed to use meaningful names. Actually,
>> that should be done regardless.
>>
>> --
>> Glynn Clements <glynn at gclements.plus.com>
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osgeo.org/pipermail/grass-dev/attachments/20140609/4a4902b8/attachment.html>