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

[Stephen F Northover]

+1

Steve

On 2013-10-21 5:46 PM, Jonathan Giles wrote:
> 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
>>
>