Bugzilla – Full Text Bug Listing
| Summary: | Comparator in Point.eliminateDuplicates() violates contract of Comparator | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Tools] GEF | Reporter: | Colin Sharples <ctg> | ||||
| Component: | GEF Geometry | Assignee: | Alexander Nyßen <nyssen> | ||||
| Status: | RESOLVED FIXED | QA Contact: | |||||
| Severity: | normal | ||||||
| Priority: | P3 | CC: | nyssen | ||||
| Version: | unspecified | Flags: | nyssen:
iplog+
nyssen: mars+ |
||||
| Target Milestone: | 3.10.0 (Mars) M6 | ||||||
| Hardware: | PC | ||||||
| OS: | Windows NT | ||||||
| Whiteboard: | |||||||
| Attachments: |
|
||||||
Thanks Colin. I will take a look. Could you please sign the Eclipse Contributor License Agreement (https://www.eclipse.org/legal/CLA.php), then we could directly adopt the test case to our code base as well. I have signed the CLA. (In reply to Colin Sharples from comment #2) > I have signed the CLA. Hmm, something does not seem to be appropriate. In the comment header, your CLA-state is indicated as not accepted. Perhaps it takes some time for the CLA status to synch through? Seems to be synched by now. Could you please confirm compliance with the Eclipse Certificate of Origin: http://www.eclipse.org/legal/CoO.php? I will then turn your test data into an appropriate test case and commit it to our repo. I confirm compliance with the CoO. (I can't see a form for the CoO, so I presume this comment is enough?) Thanks Colin. I added your fix to Point, as well as a test case (with your test-data) to PointTests. Committed changes to origin/master (signed-off by you, with me as additional author). Resolving as fixed in 3.10.0M6. |
Created attachment 251009 [details] test case with points that cause the exception For certain sets of points, the Point.getConvexHull() method will throw an IllegalArgumentException due to a comparator in method Point.eliminateDuplicates() that violates the Comparator contract. The comparator used only returns -1 or 1, but the Comparator contract states that 0 must be returned if the two objects are exactly identical. If I add the following lines before the final return, then everything works fine: if (p1.x == p2.x && p1.y == p2.y) { return 0; } I came across this when getting the outline of several shapes that had a lot of common elements, so there was a high number of duplicates to be discarded - 131 of the 168 points get discarded by the eliminateDuplicates() method (with the fix), so I guess this is a bit of an edge case, but it is best to stick to contracts. I have attached a test case with the points that generate the exception.