[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

[Glynn Clements]

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>