commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Thomas Neidhart <thomas.neidh...@gmail.com>
Subject Re: [CLI] Option class may need some attention
Date Sat, 16 Feb 2013 20:53:54 GMT
On 02/16/2013 03:04 PM, Duncan Jones wrote:
> Hi,
> 
> The Option class has some odd characteristics that I think should be addressed:
> 
> 1. The class can be constructed with null values, either by using the
> default builder constructor (discussed in CLI-224) or by passing null
> values into the constructor. I think instead we should define the
> minimum information required for a meaningful Option (short option and
> description?) and enforce that the user passes these.
> 
> Without such a change, it's possible to produce nonsense options and
> cause exceptions, e.g.
> 
>         Option option = new Option(null, null);
>         option.getId(); // throws NullPointerException
> 
> 
> In a similar vein, I think the new default constructor proposed for
> the builder in CLI-224 should go.

I am fine to add additional sanity checks for nonsense input.

> 2. The equals method only inspects the short and the long option. Even
> if this is a good idea (and I'm quite convinced it isn't), it should
> at least be documented. Otherwise, one can produce surprisingly
> "equal" objects, such as:
> 
>         Option option = new Option(null, "foo");
>         Option option2 = new Option(null, "bar");
>         assertNotEquals(option, option2); // Fails because objects are "equal"
> 
> Unless there is a good reason, the equals method should test all fields.
> 
> Any thoughts on this? Can we make such changes without breaking too
> much backwards compatibility?

The parser takes advantage of this wrt required options in some weird
way which I honestly do not want to touch as it seems to work.

The 1.x API has several flaws, and I am not sure if it is worth
correcting them. My intention is to do a 1.3 release with several
bugfixes / new features (like the new DefaultParser) and then move on to
cli 2 which will hopefully fix these problems.

Thomas

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message