[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]

Interesting, actually this works better I think.

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

G_option_exclusive(&opt1, &opt2, &flag1); // no need for the first name and
last NULL arguments.


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

> 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/e63e5658/attachment-0001.html>