[cp-patches] RFA: Component.java Fix

David Gilbert david.gilbert at object-refinery.com
Tue Jun 27 19:25:08 UTC 2006


David Gilbert wrote:

> Tom Tromey wrote:
>
>>>>>>> "Tania" == Tania Bento <tbento at redhat.com> writes:
>>>>>>>
>>
>> I'm curious about this...
>>
>> Tania> -    if (o == null)
>> Tania> -      throw new NullPointerException();
>> Tania>      ComponentOrientation oldOrientation = orientation;
>> Tania>      orientation = o;
>>
>> So the resulting orientation will be null?
>> I don't know anything about this area, so forgive me if my comments
>> are misguided.  But, this seems weird.  Does the Mauve test check
>> the result of a subsequent getComponentOrientation call?
>>  
>>
> The API spec is silent about how null is treated, so I wrote the 
> following in Mauve:
>
>    // try null
>    c.setComponentOrientation(null);
>    harness.check(c.getComponentOrientation(), null);
>    harness.check(events.size(), 1);
>    e = (PropertyChangeEvent) events.get(0);
>    harness.check(e.getSource(), c);
>    harness.check(e.getPropertyName(), "componentOrientation");
>    harness.check(e.getOldValue(), ComponentOrientation.LEFT_TO_RIGHT);
>    harness.check(e.getNewValue(), null);
>
> I don't think it makes a lot of sense to allow null, but the reference 
> implementation allows it and so I think we need to as well, to be 
> compatible.
>
>> Also, in a case like this it would be nice to update the javadoc to
>> reflect what happens if null is passed in.  With your patch it will
>> still claim to throw, which is wrong.
>>  
>>
> I agree, the patch needs to be updated to correct the API docs.
>
> Regards,
>
> Dave
>
I forgot to mention that the applyComponentOrientation() method in 
Container.java DOES throw a NullPointerException, which is better but 
inconsistent with Component.setComponentOrientation().  It turns out 
there is a bug report for the latter case:

http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4436336

...and it looks as though Sun plans to just document the behaviour 
rather than fix it (which of course is sensible for the sake of 
backwards compatibility).

Regards,

Dave



More information about the Classpath-patches mailing list