Community
Participate
Working Groups
20060920 We have an API class JavaElementSorter that extends ViewerSorter. I can't change that as it would be API breaking but I was hoping that that I could still be used as both a ViewerSorter and a ViewerComparator as ViewerSorter extends ViewerComparator . Internally I changed our code to only use getComparator when comparing elements, to avoid any use of getCollator. The APi is still there, but just for API compability Unfortunatly ViewerSorter reimplements getComparator and replaces it with getCollator. This blocks all accesses to the comparator instance in the ViewerComparator. One possibility I see is: Change ViewerSorter(Collator c) to call super(c), so no overriding of getComparator is required. In our case we would call ViewerSorter(null) and overwrite getComparator and getCollator to lazy create the collator (see other bug).
We implemented it the current way so that it was clear when you were using a ViewerSorter you would be using a java.text.Collator and when you used ViewerComparator it was clear you were using something other than java.text.Collator (well, in theory you could use the java Collator but why bother when ViewerSorter is available to use). Boris, do we want to adopt this suggested implementation? The only problem I can see with it is it can become less clear which collator is being used when clients use a ViewerSorter.
I think it would be good to deprecate getCollator and field collator and suggest to use getComparator instead.
Created attachment 50851 [details] apply patch to org.eclipse.jface Boris, here is a patch of the suggested solution - what do you think? This strategy would prevent having to create new comparator classes to replace the sorter classes that are currently API (see bug 157495).
The patch looks good to me, sorry that it took me so long to find a quiet moment to look at this. That said, I would still prefer deprecating the existing subclasses of ViewerSorter and creating new ones that only inherit from ViewerComparator. (Assuming that this does not cause disproportional amounts of work for us and for clients.) It does not seem a good practice to me to subclass ViewerSorter and then hope that everybody will just call getComparator() and never getCollator().
Duplicating the API classes will just double our maintenance as we end upm with 2 sorters that have to provide the same funtionality. I think keeping API classes that extend ViewerSorter is the best solution: just deprecate getCollator. Note that our JavaElementSorter is not intended to be extended by clients, so its only the code inside the JavaElementSorter that has to be changed (by us) to use getComparator. Ok, getCollator is even public, but there's not much reason to create a sorter and the call getCollator. -> deprecate getCollator and collator, but leave ViewerSorter non-deprecated so that our API subclasses stay clean.
Discussed with Boris and Tod. We will not deprecate ViewerSorter class to avoid potentially creating lots of warnings in subclasses and because lots of ViewerSorters may not even use a collator, but will deprecate the collator field and getter. The comment recommending use of ViewerComparator will remain in the class comment of ViewerSorter. And, of course, ViewerSorter#getComparator will be removed. UI will likely deprecate their API subclasses of ViewerSorter and provide subclasses of ViewerComparator (see bug 147495). Released for build > 20060929.
verified in I20061031-0656
