[REVIEW REQUEST] RT-33442: isSelected in TableViewSelectionModel is called too many times

[Jonathan Giles]

I've added an updated patch to RT-33442. From the jira comment included
with the patch:

1) selectRange method in TableSelectionModel is now abstract (given that
TableSelectionModel is new in 8.0 this is fine).

2) selectRange argument list is now (int, TableColumnBase, int,
TableColumnBase)

3) TableViewSelectionModel API is now unfortunately the same (i.e. I
can't make the type TableColumn, which is more specific, and requires
casting inside the method)

4) I have ported the patch to also apply to TreeTableView.

5) The same problem in 3) also exists for TreeTableView.

I'll get a implementation review done by a controls team member, but a
+1 on the API choice would be appreciated.

-- Jonathan

On 22/10/2013 9:43 a.m., Stephen F Northover wrote:
> TableSelectionModel is new for JDK8 (at least according to the
> Javadoc).  If so, add the method as abstract and put implementations
> in the concrete subclasses, no?
>
> Steve
>
> On 2013-10-21 3:15 PM, Jonathan Giles wrote:
>>
>> On 22/10/2013 2:35 a.m., Stephen F Northover wrote:
>>> 1) Is it possible to do the optimization without adding API (ugly
>>> but safe)?
>> There are aspects of the optimisation that can be done without adding
>> API. However, in my testing this API change is the most important of
>> the changes, so without it there would be some, but much less,
>> benefit of including the other changes.
>>> 2) Another alternative (ugly) is to add the API but make it return a
>>> boolean indicating whether it happened or not.
>> Sure, I'm fine with this if this is the desired approach.
>>> 3) It seem really weird to me that you can't convert from Column to
>>> index and back again.  What about getColumns()?
>> The reason why this is the case is that TableSelectionModel is
>> control independent, so it does not have the TableView or
>> TreeTableView available to it. If it did these translations would be
>> trivial (and are done all the time). This is why TableSelectionModel
>> is currently a no-op, whereas the actual TableSelectionModel
>> implementations (hidden within TableView and TreeTableView) can do
>> precisely this.
>>
>> -- Jonathan
>