Community
Participate
Working Groups
In experimenting with the new DateAndTimeObservableValue, I found the following two cases to cause NPEs in Calendar.setTime, which is invoked via DateTimeSelectionProperty: 64: Calendar cal = (Calendar) calendar.get(); 65: cal.setTime((Date) value); Here is a summary of the case: WritableValue value; DateAndTimeObservableValue dAndTObservable; Binding binding = dbc.bindValue(dAndTObservable, value); (a) value.setValue(null) (b) dAndTObservable.setValue(null); Also it is not clear to me what the expected result of dAndTObservable.getValue() is on those cases (i.e. considering the DateTime widgets do not support null values). See attached test case for details.
Created attachment 131361 [details] Test Case
DateAndTimeObservableValue inherits its constraints from the underlying date and time observables. Since as you stated DateTime does not support null selection this would also apply to DateAndTimeObservableValues backed by a DateTimeSelectionProperty observable. We should change to DateTimeSelectionProperty.doSetValue to throw a more informative exception if a null value is passed in.
Created attachment 131409 [details] Fix with tests
Fix released to HEAD > 20090409
Thanks for the clarification and fix. I'm reopening, to contribute an enhancement to the javadoc of DateAndTimeObservable related to this bug. I found the original javadoc confusing, because it gives impression that null is an acceptable value. Hope this helps, Elias.
Created attachment 131454 [details] Javadoc patch for DateAndTimeObservable
The javadoc changes in your patch are not accurate. Setting a null value does not necessarily cause a RuntimeException, even though that is a possibility. It depends on the underlying date and time observables. For example the CalendarCombo project in Nebula has a "None" button so in that case DateAndTimeObservableValue would work fine with a null value.
Created attachment 131726 [details] Javadoc patch Hi Matthew, thanks for your input. I did not consider the fact that different widgets may or may not support null. Still I think it is important to document that restrictions do exist. I've also added some clarifying remarks regarding binding to only one widget, as you suggested in Bug #169876. Attaching patch...
Released to HEAD > 20090417 Thanks Elias for the patch! And thanks for not giving up earlier :)
