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

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