I am trying to create a library of my own which contains (among others), a class called Point. As the name suggests, it is intended to encapsulate a point represented in 2D space. This is what I've come up with:
package geom;
import static java.lang.Math.atan2;
import static java.lang.Math.cos;
import static java.lang.Math.pow;
import static java.lang.Math.sin;
import static java.lang.Math.sqrt;
import static java.lang.Math.toDegrees;
import static java.lang.Math.toRadians;
/**
* A class that provides factory methods for creation of {@code Points}, utility
* functions based on operations regarding points.
* <p>
* The constructor has been made {@code private} because of the following
* reason: A {@code Point} can be initialized in <i>two</i> different ways:
* <ul>
* <li>Using the <b>Cartesian System</b> of using two variables to represent the
* coordinates of the {@code Point}.</li>
* <li>Using the <b>Polar System</b> of using the distance from the original and
* the angle between the line joining the point and the origin and the X-axis to
* represent the location of the {@code Point}.</li>
* </ul>
* Since there are <i>two</i> parameters in both cases, and each is of the type
* {@code double}, an ambiguity arises.
* <p>
* A solution to the above problem is to use <em>Factory Methods</em>.<br>
* Factory methods are {@code public static} methods which can accessed through
* class reference, like:
* <p>
* <code>Point point = Point.getCartesian(10, 20);</code><p>
* The advantages of using Factory Methods are many; a few of them are:
* <ol><li>Through the use of Factory Methods with descriptive names, the
* ambiguity is removed.</li>
* <li>These methods return a {@code new Point} if it has not been created
* before.<br>
* If a pre-existing point exists, the {@code Point} is returned from the
* {@code PointTracker}.</li>
* </ol>
*
*
* @author ambigram_maker
* @version 1.0
* @version 2014-04-02
*/
public class Point {
/*
* This variable represents the state of the output.
* It is static because any change to this variable is reflected in all the
* instances.
*/
private static State state;
/**
* The Point object representing the reference point at the intersection of
* the coordinate axes. Essentially, it is a point whose coordinates are
* <pre>(0,0)</pre>.
*/
public static final Point ORIGIN = getCartesianPoint(0, 0);
/**
* This factory method returns a {@code Point} object with the (Cartesian)
* coordinates passed as parameters.
*
* @param x The X-coordinate of the {@code Point} object.
* @param y The Y-coordinate of the {@code Point} object.
* @return The required {@code Point} object.
*/
public static Point getCartesianPoint(double x, double y) {
Point p = new Point();
p.x = x;
p.y = y;
p.radius = sqrt(x * x + y * y);
p.angleR = atan2(x, y);
p.angleD = toDegrees(p.getAngleRadians());
return p;
}
/**
* This factory method returns a {@code Point} object with the (Polar)
* coordinates passed as the parameters.
*
* @param radius The distance of the required {@code Point} from the origin
* (0,0)
* @param degrees The angle between the line joining the required
* {@code Point} and the origin and the X-axis (in degrees i.e. from 0 to
* 360).
* @return The required {@code Point} object.
*/
public static Point getPolarDegreesPoint(double radius, double degrees) {
Point p = new Point();
p.radius = radius;
p.angleD = degrees;
p.angleR = toRadians(degrees);
initPolar(p);
return p;
}
/**
* This factory method returns a {@code Point} object with the (Polar)
* coordinates passed as the parameters.
*
* @param radius The distance of the required {@code Point} from the origin
* (0,0)
* @param radians The angle between the line joining the required
* {@code Point} and the origin and the X-axis (in radians i.e. from 0 to
* 360).
* @return The required {@code Point} object.
*/
public static Point getPolarRadiansPoint(double radius, double radians) {
Point p = new Point();
p.radius = radius;
p.angleR = radians;
p.angleD = toDegrees(radians);
initPolar(p);
return p;
}
/*
* This method is common to both the 'getPolar_______Point()' methods.
*/
private static void initPolar(Point point) {
double angle = point.getAngleRadians();
point.x = point.getRadius() * cos(angle);
point.y = point.getRadius() * sin(angle);
}
/**
* This method is used to change the form of representation of <em>ALL</em>
* {@code Point} objects.
*
* @see State
* @param state The {@code State} constant to set.
*/
public static void setState(State state) {
Point.state = state;
}
/*
* The coordinates of this Point in the Cartesian system.
*/
private double x; // The perpendicular distance from the Y-axis.
private double y; // The perpendicular distance from the X-axis.
/*
* The coordinates of this Point in the Polar System.
*/
private double radius; // The distance from the Origin (0,0).
private double angleR; // The angle in Radians.
private double angleD; // The angle in Degrees.
private Quadrant location;
/*
* Instances of Point are not meant to be created through the default
* contructor. Use the Factory Methods instead.
*/
private Point() {
// Provision to add itself to the PointTracker.
}
/**
* Returns the <i>distance</i> between {@code this Point} and the one passed
* in the parameter.
*
* @param other The <i>other</i> {@code Point} which acts as the reference
* for calculating the distance.
* @return The distance {@code this Point} and the <i>other</i> one.
*/
public double distanceFrom(Point other) {
return other.equals(Point.ORIGIN) ? radius
: sqrt(pow(this.getX() - other.getX(), 2)
+ pow(this.getY() - other.getY(), 2));
}
/**
* This method returns the {@code Point} which is a reflection of
* {@code this Point} in the X-axis.
*
* @return Returns the {@code Point} which is a reflection of
* {@code this Point} in the X-axis.
*/
public Point reflectionXAxis() {
return getCartesianPoint(getX(), -getY());
}
/**
* This method returns the {@code Point} which is a reflection of
* {@code this Point} in the Y-axis.
*
* @return Returns the {@code Point} which is a reflection of
* {@code this Point} in the Y-axis.
*/
public Point reflectionYAxis() {
return getCartesianPoint(-getX(), getY());
}
/**
* This method returns the {@code Point} which is a reflection of
* {@code this Point} in the Origin.
*
* @return Returns the {@code Point} which is a reflection of
* {@code this Point} in the Origin.
*/
public Point reflectionOrigin() {
return getCartesianPoint(-getX(), -getY());
}
/**
* This method returns the {@code Point} which is a reflection of
* {@code this Point} in the {@code Point} passed as a parameter.
*
* @param other The reference for calculating the reflection of
* {@code this Point}
* @return Returns the {@code Point} which is a reflection of
* {@code this Point} in the X-axis.
*/
public Point reflectionFrom(Point other) {
if (other.equals(Point.ORIGIN)) {
return reflectionOrigin();
}
return getCartesianPoint(
2 * other.getX() - this.getX(),
2 * other.getY() - this.getY());
}
/**
* Returns the X-coordinate of {@code this Point}.
* <p>
* The magnitude of the X-coordinate is the perpendicular distance between
* {@code this Point} and the Y-axis. If {@code this Point} is above the
* X-axis, the X-coordinate is positive. If {@code this Point} is below the
* X-axis, the X-coordinate is negative.
*
* @return the X coordinate of {@code this Point}.
*/
public double getX() {
return x;
}
/**
* Returns the Y-coordinate of {@code this Point}.
* <p>
* The magnitude of the Y-coordinate is the perpendicular distance between
* {@code this Point} and the X-axis. If {@code this Point} is above the
* Y-axis, the Y-coordinate is positive. If {@code this Point} is below the
* Y-axis, the Y-coordinate is negative.
*
* @return the Y coordinate of {@code this Point}.
*/
public double getY() {
return y;
}
/**
* Returns the distance between the origin (0,0) and {@code this Point}.
* Considering the origin to be at the center and {@code this Point} at the
* circumference, this distance is the <i>radius</i>.
*
* @return The Distance between the origin and {@code this Point}.
*/
public double getRadius() {
return radius;
}
/**
* Returns the angle between the line joining {@code this Point} and the
* origin, and the X-axis in Radians.
*
* @return The angle between the line joining {@code this Point} and the
* origin, and the X-axis in Radians.
*/
public double getAngleRadians() {
return angleR;
}
/**
* Returns the angle between the line joining {@code this Point} and the
* origin, and the X-axis in Degrees.
*
* @return The angle between the line joining {@code this Point} and the
* origin, and the X-axis in Degrees.
*/
public double getAngleDegrees() {
return angleD;
}
/**
* Returns the <i>location</i> of {@code this Point} in a broader
*
* @return
*/
public Quadrant getLocation() {
if (location == null) {
if (this.equals(Point.ORIGIN)) {
location = Quadrant.ON_ORIGIN;
} else if (x == 0) {
location = Quadrant.ON_Y_AXIS;
} else if (y == 0) {
location = Quadrant.ON_X_AXIS;
} else if (x > 0 && y > 0) {
location = Quadrant.FIRST_QUADRANT;
} else if (x < 0 && y > 0) {
location = Quadrant.SECOND_QUADRANT;
} else if (x < 0 && y < 0) {
location = Quadrant.THIRD_QUADRANT;
} else if (x > 0 && y < 0) {
location = Quadrant.FOURTH_QUADRANT;
}
}
return location;
}
/**
* This method is used to check if two instances of {@code Point} are equal.
* This method checks the {@code Point}s using their hashcodes.
*
* @see Point#hashCode()
* @param o The {@code Object} to compare this instance with.
* @return {@code true} if the {@code Object} passed as parameter is an
* instance of type {@code Point} and the two {@code Point}s are
* <i>equal</i>
*/
@Override
public boolean equals(Object o) {
if (o instanceof Point) {
Point p = (Point) o;
return this.hashCode() == p.hashCode();
}
return false;
}
@Override
public int hashCode() {
int hash = 0;
hash += (int) (Double.doubleToLongBits(this.getX())
^ (Double.doubleToLongBits(this.getX()) >>> 32));
hash += (int) (Double.doubleToLongBits(this.getY())
^ (Double.doubleToLongBits(this.getY()) >>> 32));
return hash;
}
@Override
public String toString() {
Thread t = new Thread();
String summary = "\tCartesian:\t( x\t: " + x + ", y\t: " + y + " )";
if (state == null) {
setState(State.SHORT_SUMMARY);
}
if (!state.equals(State.NO_SUMMARY)) {
summary += "\n\tPolar:\n\t\tDegrees\t( radius\t: " + radius + ", angle\t: "
+ angleD + " )\n";
summary += "\t\tRadians\t( radius\t: " + radius + ", angle\t: "
+ angleR + " )\n";
}
if (state.equals(State.LONG_SUMMARY)) {
summary += "\tQuadrant\t: " + getLocation();
// summary += "\n\t" + Integer.toHexString(hashCode());
}
return summary;
}
/**
*
*/
@SuppressWarnings("PublicInnerClass")
public static enum State {
/**
* If the {@code state} of a {@code Point} is set to this value, then
* the {@code toString()} will display:
* <ol>
* <li>Cartesian Representation : (x,y)</li>
* <li>Polar Representation (r,θ) in :
* <ul><li>Degrees</li><li>Radians</li></ul></li>
* <li>The quadrant in which the {@code Point} is located.</li>
* </ol>
*/
LONG_SUMMARY,
/**
* If the {@code state} of a {@code Point} is set to this value, then
* the {@code toString()} will display:
* <ol>
* <li>Cartesian Representation : (x,y)</li>
* <li>Polar Representation (r,θ) in :
* <ul><li>Degrees</li><li>Radians</li></ul></li>
* </ol>
*
*/
SHORT_SUMMARY,
/**
* If the {@code state} of a {@code Point} is set to this value, then
* the {@code toString()} will display the Cartesian Representation :
* (x,y) of this {@code Point}.
*/
NO_SUMMARY;
}
@SuppressWarnings("PublicInnerClass")
public static enum Quadrant {
FIRST_QUADRANT,
SECOND_QUADRANT,
THIRD_QUADRANT,
FOURTH_QUADRANT,
ON_X_AXIS,
ON_Y_AXIS,
ON_ORIGIN;
}
}
I used NetBeans 8.0 to create the above class, so the arrangement, the warning suppression has been suggested by this software. I need people to criticize and discuss upon:
- The effectiveness of the code.
- The organization of the code.
- Possible ways to improve the performance, readability, simplicity, etc.
- Is the hashing good enough?
- Any errors in the documentation (technical, accidental) of the code.
- Any other aspect focused upon improvement of my programming skills.
through downvotes (and upvotes?1), comments and answers (of course!). Please note that this is a library class that anyone can use or modify, but inform me first.
EDIT:
After [encountering] so many varied answers, I have decided to change these aspects:
- As suggested by RoToRa in this answer, I'll get rid of the
Stateidea completely, because it is actually temporary. - From the same answer, I'll change the name of the factory methods to start with
create.... - Fix the bug pertaining to the angle (from the same answer).
- Modify the
hashcode()andequals()method. (I have done it here). - As suggested by coredump in this answer, I'll change
initPolar()(or maybe even get rid of it). - As suggested by Eike Schulte in this answer, I'll fix the
atan2(y,x)bug. - As suggested by ChrisW over here, I'll use
pow(..)throughout. - Use a 5-parameter constructor (for use by the factory methods).
(These changes haven't been applied to the code above.)
- In addition to these, I'm thinking of adding a
scalevariable (double) that is used to decide the degree of accuracy (and overcome the floating-point issues). Is it really practical? - Justification for
PointTracker:
I'll add a post later on about the classPointTrackerafter some time. This class is (as the name suggests) supposed to track (all?) thePointscreated during runtime. It'll add support for yet another library class:Line. (Hence, you can expect quite a few posts related to this topic.) - Last, but not the least I invite more answers pertaining to topics other than those resolved above, so that this
Pointis the ideal class. I also invite demonstrations of just-in-time implementations of the inter-conversion of the angles or other related operations.
Please guys, remember that this is for you; so give suggestions to make it more personal.
1 : Help CodeReview graduate!
equalsdepending onhashCodeis very bad. We would not expect(1, 0)and(0, 1)to be considered equal and right now they are. \$\endgroup\$reflectionFrom? \$\endgroup\$this PointandPoint.ORIGINis already stored -->radius". But then again, I don't know if it's practical. \$\endgroup\$finalthanks to all the generous people of CodeReview. :-) \$\endgroup\$