Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 158118

Summary: Change ViewerSorter.getComparator?
Product: [Eclipse Project] Platform Reporter: Martin Aeschlimann <martinae>
Component: UIAssignee: Karice McIntyre <Karice_McIntyre>
Status: VERIFIED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: bokowski, Tod_Creasey
Version: 3.2   
Target Milestone: 3.3 M3   
Hardware: PC   
OS: Windows XP   
Whiteboard:
Bug Depends on:    
Bug Blocks: 157495    
Attachments:
Description Flags
apply patch to org.eclipse.jface none

Description Martin Aeschlimann CLA 2006-09-21 05:21:39 EDT 
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).
Comment 1 Karice McIntyre CLA 2006-09-22 14:55:16 EDT 
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.
Comment 2 Martin Aeschlimann CLA 2006-09-25 04:21:33 EDT 
I think it would be good to deprecate getCollator and field collator and suggest to use getComparator instead.
Comment 3 Karice McIntyre CLA 2006-09-25 14:31:44 EDT 
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).
Comment 4 Boris Bokowski CLA 2006-09-28 13:38:42 EDT 
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().
Comment 5 Martin Aeschlimann CLA 2006-09-28 17:33:23 EDT 
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.
Comment 6 Karice McIntyre CLA 2006-09-29 10:50:47 EDT 
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.
Comment 7 Karice McIntyre CLA 2006-10-31 11:56:50 EST 
verified in I20061031-0656