On 02/16/2013 10:18 PM, Duncan Jones wrote:
> On 16 February 2013 20:53, Thomas Neidhart <thomas.neidh...@gmail.com> wrote:
>> 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.
> 
> OK. I would suggest the short option is the only required parameter
> (non-null, non-empty) - a description should be optional. What do you
> say?

You either have to specify a short or long option (or both of course).
The old OptionBuilder actually checks this condition in the create
method. When creating an Option yourself, this can not be checked in the
constructor, as you may call setLongOption afterwards.

This should be added to the javadoc and the new Option.Builder should be
adapted accordingly.

There is no need to create a separate issue for this, I will do the
necessary changes.

Thomas

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

Reply via email to