patternjavaModerate
A Library Class : Point
Viewed 0 times
classlibrarypoint
Problem
I am trying to create a library of my own which contains (among others), a
```
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.
*
* The constructor has been made {@code private} because of the following
* reason: A {@code Point} can be initialized in two different ways:
*
* Using the Cartesian System of using two variables to represent the
* coordinates of the {@code Point}.
* Using the Polar System 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}.
*
* Since there are two parameters in both cases, and each is of the type
* {@code double}, an ambiguity arises.
*
* A solution to the above problem is to use Factory Methods.
* Factory methods are {@code public static} methods which can accessed through
* class reference, like:
*
* Point point = Point.getCartesian(10, 20);
* The advantages of using Factory Methods are many; a few of them are:
* Through the use of Factory Methods with descriptive names, the
* ambiguity is removed.
* These methods return a {@code new Point} if it has not been created
* before.
* If a pre-existing point exists, the {@code Point} is returned from the
* {@code PointTracker}.
*
*
*
* @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
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.
*
* The constructor has been made {@code private} because of the following
* reason: A {@code Point} can be initialized in two different ways:
*
* Using the Cartesian System of using two variables to represent the
* coordinates of the {@code Point}.
* Using the Polar System 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}.
*
* Since there are two parameters in both cases, and each is of the type
* {@code double}, an ambiguity arises.
*
* A solution to the above problem is to use Factory Methods.
* Factory methods are {@code public static} methods which can accessed through
* class reference, like:
*
* Point point = Point.getCartesian(10, 20);
* The advantages of using Factory Methods are many; a few of them are:
* Through the use of Factory Methods with descriptive names, the
* ambiguity is removed.
* These methods return a {@code new Point} if it has not been created
* before.
* If a pre-existing point exists, the {@code Point} is returned from the
* {@code PointTracker}.
*
*
*
* @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
Solution
Use use of a "state" to distinguish between different output formats is very wrong. If you need different formats, use multiple output methods (have
I think you have overdone it with the JavaDoc. IMHO it's neither the place for theoretical excursus on how coordinate systems work, nor the place to discuss implementation decisions ("Since there are two parameters in both cases, and each is of the type {@code double}, an ambiguity arises.").
The names of the factory methods shouldn't begin with
You should modulo the degrees to 360 (and the radians to 2π) otherwise you get:
The use of the hash code inside the equals not good. One of the functions of
toString just for debug output), or even better, write a separate class for converting Points to a string representation.I think you have overdone it with the JavaDoc. IMHO it's neither the place for theoretical excursus on how coordinate systems work, nor the place to discuss implementation decisions ("Since there are two parameters in both cases, and each is of the type {@code double}, an ambiguity arises.").
The names of the factory methods shouldn't begin with
get... since they are not getters. Use create... instead.You should modulo the degrees to 360 (and the radians to 2π) otherwise you get:
Point p1 = Point.getPolarDegreesPoint(10,45);
Point p2 = Point.getPolarDegreesPoint(10,405);
System.out.println(p1.equals(p2)); // prints false, but should be true.The use of the hash code inside the equals not good. One of the functions of
equals is to distinguish between two objects with the same hash code - which will happen, considering you are "squeezing" two 64 bit double values into a 32 bit integer.Code Snippets
Point p1 = Point.getPolarDegreesPoint(10,45);
Point p2 = Point.getPolarDegreesPoint(10,405);
System.out.println(p1.equals(p2)); // prints false, but should be true.Context
StackExchange Code Review Q#46152, answer score: 16
Revisions (0)
No revisions yet.