HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Finding nearest points and points within a radius

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
nearestpointswithinradiusfindingand

Problem

I have implemented the following two methods which are described in 1. and 2..

Is there another better simpler way to implements those methods?



-
A static method, nearestPoint , accepts an array of points (objects of type Point ) and one point (an object of type Point),
and returns that point in the array which is closest to the given
point. Create that method.

-
A circle in the plane, with radius \$r\$ and midpoint at the origin, is given by the following equation: \$x^2 + y^2 = r^2\$


A static method, internalPoints , accepts an array of points (objects
of type Point) and the radius of a circle, and returns those points
(in an array) that are inside the given circle. Create that method.

-
Create an array of points (objects of type Point) and a point (an object of type Point). Establish also the radius of the circle.

Use the method nearestPoint to determine the point in the array that
is closest to the given point. Then use the method internalPoints to
determine the points that are inside the circle.


```
public class Point {
// the coordinates of the point
private double x;
private double y;

public Point(double x, double y) {
this.x = x;
this.y = y;
}

public double getX() {
return this.x;
}

public double getY() {
return this.y;
}

// distance returns the distance between this point and a given point
public double distance(Point p) {
return Math.sqrt((p.x - this.x) * (p.x - this.x) +
(p.y - this.y) * (p.y - this.y));
}

public String toString() {
String s = "";
//for (int i = 0; i < s.length(); i++) {
s = "(" + this.getX() + "," + this.getY() + ")";

return s;
}

public static Point nearestPoint(Point[] points, Point point) {
Point p = points[0];
for (int i = 0; i < points.length; i++) {
if (points[i].distance(point) < p.distance(point)) {
p = points[i];
}
}
return p;
}

public static Point[] internalPoints(Point

Solution

Bug

The internalPoints method has a bug:
you count the number of points that fall within the radius,
but then, instead of adding those points in the output array,
you take the first N points of the input array,
some of which may not be within the radius.
Try for example with this input:

Point[] points = {
        new Point(1, 2),
        new Point(19, 29),
        new Point(2, 3),
        new Point(5, 2)
};


Comparing distances

Do you really need to compare \$\sqrt{dx^2 + dy^2}\$ ?
Computing the square root can be expensive,
and comparing \$dx^2 + dy^2\$ would be enough.

public double dsquare(Point p) {
    double dx = p.x - this.x;
    double dy = p.y - this.y;
    return dx * dx + dy * dy;
}


Finding the nearest point

In this loop, the distance between point and the nearest point may be repeatedly recalculated:

Point p = points[0];
for (int i = 0; i < points.length; i++) {
    if (points[i].distance(point) < p.distance(point)) {
        p = points[i];
    }
}
return p;


You could save the known nearest distance in a variable to avoid repeated computations. Also, you could start iterating from i = 1 instead of i = 0:

Point nearest = points[0];
double nearestDistance = point.dsquare(nearest);
for (int i = 1; i < points.length; i++) {
    double distance = points[i].dsquare(point);
    if (distance < nearestDistance) {
        nearest = points[i];
        nearestDistance = distance;
    }
}
return nearest;


Simplify

The s variable in toString is pointless, you could directly return the concatenated value.

Code Snippets

Point[] points = {
        new Point(1, 2),
        new Point(19, 29),
        new Point(2, 3),
        new Point(5, 2)
};
public double dsquare(Point p) {
    double dx = p.x - this.x;
    double dy = p.y - this.y;
    return dx * dx + dy * dy;
}
Point p = points[0];
for (int i = 0; i < points.length; i++) {
    if (points[i].distance(point) < p.distance(point)) {
        p = points[i];
    }
}
return p;
Point nearest = points[0];
double nearestDistance = point.dsquare(nearest);
for (int i = 1; i < points.length; i++) {
    double distance = points[i].dsquare(point);
    if (distance < nearestDistance) {
        nearest = points[i];
        nearestDistance = distance;
    }
}
return nearest;

Context

StackExchange Code Review Q#158758, answer score: 5

Revisions (0)

No revisions yet.