Hi Semyon,
On 29/06/2017 02:51, Semyon Sadetsky wrote:
Hello Luke,
DefaultButtonModel ::getGroup has never been being required to return
not-null. Can you clarify which new pitfall is introduced by the fix,
so that we should specify it explicitly?
--Semyon
Code using a ButtonModel might assume that after calling setGroup that
the same instance would thereafter be returned by getGroup. However any
code taking advantage of getGroup having being "pulled up" into
ButtonModel should not make that natural assumption, as a legacy
ButtonModel may still return null.
So this forms part of the method's contract for the caller too. If the
caller is expected to read and understand the implications of the
implSpec, then what is written already may be sufficient. I guess that's
the question.
How about these changes (based on webrev.05)?
/**
* Returns the group that the button belongs to.
* Normally used with radio buttons, which are mutually
* exclusive within their group.
*
* @implSpec The default implementation of this method returns {@code
null}.
- * Subclasses should return the group set by setGroup() to avoid NPE.
+ * Subclasses should return the group set by setGroup().
*
- * @return the <code>ButtonGroup</code> that the button belongs to
+ * @return the <code>ButtonGroup</code> that the button belongs to, if any.
+ * Null may also be returned where a legacy ButtonGroup inherits the
+ * default implementation.
* @since 10
*/
default ButtonGroup getGroup() {
return null;
}
I think this makes the contract more explicit.
Kind regards,
Luke
On 06/28/2017 08:29 AM, Luke wrote:
Hi Semyon,
On 28/06/2017 16:59, Semyon Sadetsky wrote:
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().
The spec can of course only apply to code written in Java 10+, legacy
implementations ButtonModel will still return null.
So say a helper function in a shared library does an "instanceof
DefaultButtonModel" check, and that it calls both
setGroup(ButtonGroup) and getGroup() - and for whatever reason pulls
the group back out rather than storing it in a local variable.
When porting that library to Java 10, a programmer might think
"great, now I can support all ButtonModels" and drop the instanceof
check.
Some other app not yet ported to Java 10 passes that utility method a
custom ButtonModel, as it has always done. Instead of the utility
function doing nothing with it, it now throws a NPE.
I just raised this because I know how ultra-concerned the JDK is with
compatibility - but even by the JDK's high standards it does still
seem like a fairly theoretical-only issue.
Is the @implSpec already sufficient warning to a programmer porting
code to Java 10 to always deal with the null possibility? Perhaps it is.
Kind regards,
Luke
