Hello Pankaj,
Please find my inputs:
At line 1036 number of characters per line is more than 80, we can divide the
conditions into multiple lines.
1036 if (!sorted || viewToModel.length == 0 || (lastRow - firstRow) >
viewToModel.length / 10) {
1037 // We either weren't sorted, or to much changed, sort it all or
1038 // this is the first row added and we have to update different
1039 // caches
Test case is working fine before and after the change.
I think there is no need for us to verify row index before we delete and update
the contents again.
65 for (int row = 0; row < model.getRowCount(); row++) {
66 // values are in sorted ascending. so the row index and
67 // view index from sorter should be same
68 if (row != rowSorter.convertRowIndexToView(row)) {
69 throw new RuntimeException("Wrong sorting");
70 }
71 }
We can remove the above code in the test case because we never hit it and throw
RuntimeException.
Thanks,
Jay
From: Pankaj Bansal
Sent: Wednesday, January 17, 2018 5:24 PM
To: swing-dev@openjdk.java.net; Sergey Bylokhov; Semyon Sadetsky
Subject: <Swing Dev> [11] Review Request: JDK-7007967 DefaultRowSorter:
incorrect sorting due to not updating comparator cache
Hi All,
Please review the fix for JDK 11.
Bug:
https://bugs.openjdk.java.net/browse/JDK-7007967
Webrev:
http://cr.openjdk.java.net/~pbansal/7007967/webrev.00/
Issue:
The caching flags like useToString, comparators etc are not always updated in
shouldOptimizeChange function. It can cause wrong cache to be used in some
cases and produce wrong sorting results.
For example, in the submitted test case, the integer data is added and sorted.
This updates the useToString etc cache flags according to the data type
provided. //Sorts 1,2,11 as 1->2->11 (correct).
Then all the data is removed from the model and when the sorter is indicated of
this, the cache flags get updated accordingly. Because of this, the useToString
is set to true for the column and comparator is set to String Comparator and
sorted flag is still true.
Now if Integer data rows are again added one by one, these flags are not
updated and it will use the wrong caches of useToString, comparators etc and
produce wrong sorting results. //Sorts 1,2,11 as 1->11->2 (Incorrect. The data
is sorted according to String representation).
Fix:
There can be different approaches to solve this issue. In the current approach,
changes have been done in shouldOptimizeChange function to call the sort
function when first row is added to the model. This will ensure that the cache
flags like useToString, comparators are updated according to the data added for
the column.
Regards,
Pankaj Bansal
