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

[Jonathan Giles]

On 19/10/2013 8:24 a.m., Jonathan Giles wrote:
> On 19/10/2013 2:13 a.m., Stephen F Northover wrote:
>> If it is a noop, will it not break people when the system calls it and
>> it does nothing?
> Only the abstract TableSelectionModel is a no-op, the implementations
> (one each for TableView and TreeTableView) will both override and
> provide the implementation that I already have in the patch for
> TableView. I would rather have the implementation in the base class but
> this is not feasible due to requirements in looking up column indices.
I looked into providing a default implementation in TableSelectionModel,
but the problem is is that there just isn't enough data at this level of
the API.

In short, the API could take two forms:
selectRange(int minRow, int minColumn, int maxRow, int maxColumn)
selectRange(int minRow, TableColumnBase minColumn, int maxRow,
TableColumnBase maxColumn)

Either implementation is possible (although at present I've implemented
the first approach, but for consistency it is probably better we use the
second approach as all other API uses TableColumn* to represent columns,
not ints).

The problem is is that neither approach gives me what I want for a
default implementation, which would take the following form:
for (int row = minRow; row <= maxRow; row++) {
    for (int col = minColumn; col <= maxColumn; col++) {
        TableColumnBase column = getColumn(col);
        select(row, column);
    }
}

With the first API option (all ints), I am lacking the getColumn(..)
method to convert column indices to TableColumn instances.
With the second API option, I have no way of converting the two
TableColumnBase instances back into ints, and then I have the same
problem as with the first API.

Therefore, my options as far as I can see (and I'm happy to be wrong)
are to either not introduce this API and keep the performance poor (for
now - hopefully we'll come up with a better solution in time) or to
introduce one of the two methods above and then have a no-op
implementation clearly documented in TableSelectionModel (with the two
existing implementations in JavaFX 8.0 both implementing a far more high
performance approach than we have currently).

Thoughts?
-- Jonathan