feat: add closest pair of points algorithm #1706
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1706 +/- ##
==========================================
+ Coverage 84.65% 84.78% +0.12%
==========================================
Files 378 379 +1
Lines 19744 19904 +160
Branches 2951 2978 +27
==========================================
+ Hits 16715 16875 +160
Misses 3029 3029 ☔ View full report in Codecov by Sentry. |
| * @see {@link https://en.wikipedia.org/wiki/Closest_pair_of_points_problem} | ||
| * @class | ||
| */ | ||
| export default class ClosestPair { |
There was a problem hiding this comment.
This should not be a class. Instead, it should be simply an exported function which takes an array of points and returns the closest pair. All methods here should be just functions or closures inside that function.
The tests then simplify to testing that single function.
| } | ||
|
|
||
| /** | ||
| * Finds the closest pair of points in a small set (3 or fewer). |
There was a problem hiding this comment.
The "3 or fewer" is just how this is used. This method supports arbitrarily many points (but of course has quadratic time complexity). I would document this as "to be used with small sets"; there is no strict requirement here.
This "brute force" function could also be reused for testing with random points.
| } | ||
|
|
||
| /** | ||
| * Finds the closest pair in a strip of points sorted by y-coordinate. |
There was a problem hiding this comment.
This comment is wrong. Since the function sorts by Y coordinate, it is not a requirement that the strip be sorted by Y coordinate. In fact it won't be, because it's sorted by X coordinate.
| /** | ||
| * Gets the strip of points within a certain distance from a midpoint. | ||
| * @private | ||
| * @param {Array<{x: number, y: number}>} points - An array of sorted points. |
| * @returns {Array<{x: number, y: number}>} The filtered strip of points. | ||
| */ | ||
| #getStripPoints(points, midPoint, minDistance) { | ||
| return points.filter( |
There was a problem hiding this comment.
It doesn't matter for the asymptotic worst case and might make the code a bit harder to follow, but this can be optimized by leveraging the fact that the points are sorted by X and thus doing a binary search to find the bounds.
| let pair = [] | ||
|
|
||
| // Sort by y-coordinate | ||
| strip.sort((a, b) => a.y - b.y) |
There was a problem hiding this comment.
This probably implies a time complexity of
| expect(result.distance).toBe(0) // Distance between identical points is zero | ||
| }) | ||
|
|
||
| test('handles large number of points correctly', () => { |
There was a problem hiding this comment.
This isn't actually tested.
As said, I would recommend testing against the brute force search.
|
😕 |
Describe your change:
Checklist:
Example:
UserProfile.jsis allowed butuserprofile.js,Userprofile.js,user-Profile.js,userProfile.jsare notFixes: #{$ISSUE_NO}.