Hello,
On 06/23/2017 03:07 AM, Luke Usherwood wrote:
Hi,
Sorry for the late reply - I've been travelling. As the bug reporter,
would you mind if I gave a few passing thoughts?
On 6/20/2017 11:16 PM, Semyon Sadetsky wrote:
> Hi Prasanta,
>
>So, it is better check whether the model is a DefaultButtonModel and
skip if grouping is not supported. Or, perhaps, it is reasonable to
pull up the > getGroup() into the ButtonModel interface which already
has setGroup().
On one hand, it seems the best default methods are the ones able to
produce correct behaviours w.r.t. the other interface methods when
retrofitted, to avoid any 'surprises'. For example if some other code
decides to switch from DefaultButtonModel to accept any ButtonModel -
that would happily compile but it might be easy to miss the changed
semantics: that calling getGroup may not necessarily give back a value
passed in to setGroup any more, and hence there's scope for a subtle
runtime crash bug to be introduced.
Not sure that I fully understood this. According to the spec there is
the only group the button belongs to, and this group should be returned
in getGroup().
Also, would the change that pulls up a new default method into
ButtonModel be permitted if this fix were back-ported to JDK-9?
No. JDK9 backport should be different since no API changes are allowed
anymore.
On the other hand I suppose there's no other choice if you want to
patch this oversight in the interface design (which looks like it was
wanted since Java 1.3!) ... so this is the trade-off you've taken. The
likelihood of an actual crash in practice is low, and more so because
most calling code would test for a null value anyway. So I guess on
balance this may be reasonable.
If you do proceed with default method solution...
1) should the javadoc for state that all implementers are expected to
override the default implementation of getGroup(), and callers should
always test for a null value (even when that might not be expected) to
account for legacy implementations where the method might not be
implemented?
I think nothing was changed here. Before the fix there was possibility
for setGroup(null).
2) will you change similar code in the JDK - such as found in
JToggleButton.getGroupSelection() - to avoid the check-and-cast to
DefaultButtonModel. This would seem required to ensure that all
ButtonModels implementing getGroup() are treated consistently to other
DefaultButtonModel instances.
That is good point. Group selection can be supported for custom
ButtonGroup implementations as well.
@Prasanta: it is up to you to file another bug or do this as a part of
the current fix.
--Semyon
Kind regards,
Luke
