RFR: 8235491: Tree/TableView: implementation of isSelected(int) violates contract [v2]

[Jeanette Winzenburg]

On Thu, 21 Jul 2022 21:10:10 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

>> 1. reword SelectionModel.isSelected(int) javadoc, removing incorrect statement "Is functionally equivalent to calling <code>getSelectedIndices().contains(index)</code>." 
>> 2. reimplement TableView.TableViewSelectionModel.isSelected(int) method to return true when at least one cell in *any* column is selected on the given row (was: *all* columns)
>> 3. change selectRowWhenInSingleCellSelectionMode() and selectRowWhenInSingleCellSelectionMode2() in TableViewSelectionModelImplTest to reflect new reality.
>> 
>> NOTE: proposed change alters semantics of isSelected(int) method (in the right direction, in my opinion).
>
> Andy Goryachev has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - 8235491: whitespace
>  - 8235491: javadoc

modules/javafx.controls/src/main/java/javafx/scene/control/SelectionModel.java line 204:

> 202:      * in this SelectionModel.  It will return true when at least one cell
> 203:      * is selected in the specified row index.
> 204:      *

Well, I would go a step (actually 2 steps :) further:

A:

- the doc error was that a method in the base class specifies its behavior with api from a sub: can't and should not be done because it doesn't know its subs plus there are subs that don't have such api (like SingleSelection model). 
- the fix is to replace the description with a similar error: there is no notion of "cells in a row" in the base and also not in many of its subs

I would suggest to simply remove the old sentence without replacing it with anything: this base class has the notion of multiple selections in one dimension (index, row) without actually api to access those multiples, selected could stand for its own and might be fleshed out by subclasses.

B: 

we need a CSR anyway, so might cleanup a bit further: this method is _not_ a convenience method, quite on the contrary - for clients coding against this base class, it's the _only_ means to get hold of the selected indices (by looping through all indices in the valid range and calling it)

-------------

PR: https://git.openjdk.org/jfx/pull/839