Description Timo Kinnunen 2015-06-06 08:11:45 EDT

Using a debug aid function to check that Comparators implement their API contract correctly reveals that org.eclipse.jdt.internal.ui.workingsets.WorkingSetComparator takes a shortcut in its implementation, resulting in a violation.

The API contract states: "The implementor must ensure that sgn(compare(x, y)) == -sgn(compare(y, x)) for all x and y." 

When x and y are the same the only value A for which Integer.signum(A) == -Integer.signum(A) is 0.

Details from the debug aid method:
message: compare(x, x) is not 0, it is -1.

x was Other Projects (org.eclipse.ui.internal.WorkingSet) at 0
this: org.eclipse.jdt.internal.ui.workingsets.WorkingSetConfigurationDialog$12$1
comparator: org.eclipse.jdt.internal.ui.workingsets.WorkingSetComparator
array:
	Other Projects (org.eclipse.ui.internal.WorkingSet)
	All (org.eclipse.ui.internal.WorkingSet)
	Evening (org.eclipse.ui.internal.WorkingSet)
	GWT (org.eclipse.ui.internal.WorkingSet)
	Java Working Set (org.eclipse.ui.internal.WorkingSet)
	Repositories (org.eclipse.ui.internal.WorkingSet)
	Resources Working Set (org.eclipse.ui.internal.WorkingSet)
	Reviews (org.eclipse.ui.internal.WorkingSet)


Examining the WorkingSetComparator code reveals two short-circuiting if-statements. To fix the implementation these if-statements could be replaced with the following code:

		if (fIsOtherWorkingSetOnTop) {
			int result =
				Boolean.compare(IWorkingSetIDs.OTHERS.equals(w2.getId()), IWorkingSetIDs.OTHERS.equals(w1.getId()));
			if(result != 0) {
				return result;
			}
		}