On 01/31/2013 12:01 AM, Jörg Schaible wrote: > sebb wrote: > >> On 30 January 2013 18:18, Jörg Schaible <joerg.schai...@gmx.de> wrote: >>> Hi, >>> >>> sebb wrote: >>> >>> [snip] >>> >>>>>> /** a map of the required options */ >>>>>> + // N.B. This can contain either a String (addOption) or an >>>>>> OptionGroup (addOptionGroup) >>>>>> + // TODO this seems wrong >>>>>> private List<Object> requiredOpts = new ArrayList<Object>(); >>>>>> >>>>> >>>>> Indeed, I also spotted this and failed to resolve it, as the logic in >>>>> the parsers is somehow taken advantage of it in a way I do not yet >>>>> fully understand. >>>> >>>> Me neither. >>>> >>>> Maybe the code would still work if the entries were always OptionGroups. >>>> This could perhaps be done by converting the Option into a >>>> single-entry OptionGroup and storing that, rather than storing the >>>> Option key String. >>>> In theory that might work ... >>> >>> Or create a package local marker interface OptionEntry and let both >>> classes implement it. >> >> Not possible, because String is final. >> >> One might try to store the Option instead of its key (String). >> >> However, the key is used to avoid duplicate entries - addOption() >> removes any entry with a matching key. >> This would be difficult to do with Option entries. >> Option instances with equal keys are not necessarily equal, though the >> reverse is true. >> >> Also Option instances are not immutable (in fact the key is not even >> immutable - longOpt can be changed after instantiation). >> All a bit messsy. > > IMHO, the *intent* was clear. With the following diff we could introduce an > OptionEntry, although none of the mentioned problems of an Option instance > is really solved (all tests run, but it's not completely binary compatible):
[snip] I think for 1.3 we have to keep it as it is. The required options can be retrieved with a public/protected method in the Options and Parser class. Thus if we decide to change the content of the list, it will break compatibility. Thomas --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
