commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Duncan Jones <dun...@wortharead.com>
Subject [CLI] Option class may need some attention
Date Sat, 16 Feb 2013 14:04:37 GMT
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: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message