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.
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?
Duncan
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
