Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 264722 - Get rid of duplicate working set comparators
Summary: Get rid of duplicate working set comparators
Status: RESOLVED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.5   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: 3.5 M6   Edit
Assignee: Raksha Vasisht CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-02-12 12:09 EST by Dani Megert CLA
Modified: 2009-02-16 08:02 EST (History)
0 users

See Also:

Attachments
Fixed. Merged the two working set comparators into one class. (12.82 KB, patch)
2009-02-13 08:48 EST, Raksha Vasisht CLA 		daniel_megert: review-
Details | Diff
Made the minor changes. (13.30 KB, patch)
2009-02-16 07:41 EST, Raksha Vasisht CLA 		daniel_megert: iplog+
daniel_megert: review+
Details | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Dani Megert CLA 2009-02-12 12:09:04 EST 
N20090212-2000.

We currently have two working set comparators (WorkingSetSortComparator and WorkingSetComparator). We should remove one and make the remaining one configurable whether to sort the "other" working set on top. The remaining one should live in org.eclipse.jdt.internal.ui.workingsets.
Comment 1 Raksha Vasisht CLA 2009-02-13 08:48:35 EST 
Created attachment 125637 [details]
Fixed. Merged the two working set comparators into one class.
Comment 2 Dani Megert CLA 2009-02-16 04:00:48 EST 
Looks good except the new WorkingSetComparator class:
- strange formatting (some methods indented but shouldn't be)
- superfluous @since 3.5 tags (if the class itself is new and has @since 3.5 then
  it is not needed to flag each member
- missing constructor comments
- use && instead of two ifs:
  if (fIsOtherWorkingSetOnTop) {
	if (OthersWorkingSetUpdater.ID.equals(workingSet.getId()))
Comment 3 Raksha Vasisht CLA 2009-02-16 07:41:59 EST 
Created attachment 125777 [details]
Made the minor changes.
Comment 4 Dani Megert CLA 2009-02-16 08:02:04 EST 
Thanks for the patch.

Fixed in HEAD.
Available in builds > N20090215-2000.