Learn to Code
John F. Dumas
contact me | resume | how it works | example programs | testimonials | main page
Feedback Examples: Four
one | two | three | four | five
► PROGRAMMING LANGUAGE:: Java
► ASSIGNMENT: Your assignment is to write a 'Point' class. It should have these private member variables:
double mX; double mY;
And getter/setter methods for each member variable. There needs to be 3 ways to construct a 'Point' object:
Also, your 'Point' class needs to support these member functions:
public double getAngleDegrees(); public double getAngleRadians(); public double distance(double x, double y); public double distance(Point other); public String toString();
Provide a 'main' function that exercises all features of your 'Point' class.
► FEEDBACK: A pretty solid effort in general, your program did produce the right results in each of the test cases you provided so my comments won't be about functionality, more about style and design options.
The assignment stipulated that your 'Point' class needed to have two private, double member variables (mX and mY) and accessor methods for each variable. This part was right on track, exactly what I was looking for.
Next, the problem specified that there should be three ways to construct a 'Point' object:
This also worked fine and I see you addressed the ambiguity between options (1) and (2) by adding a boolean 'isPolar' flag to the constructor. That approach is pretty good but it does burden the caller with having to pass 'true' or 'false' to the constructor each time. If you're going to go this way, I'd suggest overloading things so that one of the constructors defaults to non-polar, as in:
public Point(double valueOne, double valueTwo, boolean isPolar)
{
if(!isPolar)
{
mX = valueOne;
mY = valueTwo;
}
else
{
mX = valueOne * Math.cos(valueTwo);
mY = valueOne * Math.sin(valueTwo);
}
}
public Point(double valueOne, double valueTwo)
{
this(valueOne, valueTwo, false);
}
Note that above, we used 'this' to feed the values from the second constructor right into the first one. This is an excellent habit to get into generally as it allows us to have the "one true" location where our class gets initialized (which avoids the problem of different constructors initializing things is an inconsistent manner).
However, there is another way to address this issue (when the number of parameters and their types are the same), it is called the "named constructor idiom". Here's how we can use that idiom to satisfy the problem's requirements:
public Point(double x, double y)
{
mX = x;
mY = y;
}
public Point()
{
this(0.0, 0.0);
}
public static Point fromPolarCoordinates(double radius, double theta)
{
double x = radius * Math.cos(theta);
double y = radius * Math.sin(theta);
return(new Point(x, y));
}
Since 'fromPolarCoordinates' isn't a constructor (it is in fact, a static method) we now have a totally unambigous path for making a Point object from a radius and angle. Again, notice that these expressions:
mX = x; mY = y;
Now appear *exactly one time* in the source code rather than in multiple places. One very good reason for doing this is to avoid a scenario where in one constructor we have:
mX = x; mY = y; And (say) in aother constructor we have: mX = x; mX = y; ^^ | +----------------- a typo, this was intended to be: "mY = y"
So, by repeating that code, you now have 2 or 3 additional chances to make a typo like the one above, best to avoid that problem altogether and have a single, unified way for initialzing your object.
Next, your class was to provide member functions for determining the angle your 'Point' makes with the origin. The methods were:
public double getAngleDegrees(); public double getAngleRadians();
And here is the code you provided for those methods:
public double getAngleDegrees()
{
double radians = Math.atan2(mY, mX);
return(radians * 180.0 / Math.PI);
}
public double getAngleRadians()
{
double radians = Math.atan2(mY, mX);
return(radians);
}
One thing to notice is that we make a call to 'Math.atan2' in two different places. Another issue is that we do the conversion from radians to degrees manually (by multiplying by 180 / PI). We can use a built-in java function to convert radians to degrees and reduce code duplication by doing something like this:
public double getAngleDegrees()
{
return(Math.toDegrees(getAngleRadians()));
}
public double getAngleRadians()
{
return(Math.atan2(mY, mX));
}
As above, notice how we implemented 'getAngleDegrees' in terms of our 'getAngleRadians' function (and the built-in 'toDegrees' method). Now, we no longer "own" the conversion from radians to degrees and there's just a single call to 'Math.atan2' in the code.
You were also asked to author two 'distance' methods, here is the code you wrote:
public double distance(double x, double y)
{
double xDelta = (x - mX);
double yDelta = (y - mY);
return(Math.sqrt(xDelta * xDelta + yDelta * yDelta));
}
public double distance(Point other)
{
double xDelta = (other.mX - mX);
double yDelta = (other.mY - mY);
return(Math.sqrt(xDelta * xDelta + yDelta * yDelta));
}
This code is correct (produces the right results), but similarly to the angle examples above has some duplicated code that we can remove. Notice that the delta calcuations and the sqrt call show up in both methods, we can avoid that duplication by refactoring:
public double distance(double x, double y)
{
double xDelta = (x - mX);
double yDelta = (y - mY);
return(Math.sqrt(xDelta * xDelta + yDelta * yDelta));
}
public double distance(Point other)
{
return(distance(other.mX, other.mY));
}
As a point of interest, you could also have implemented the distance calculation in the second version (that takes a single 'Point' parameter) and "fed" the first version into the second as in:
public double distance(double x, double y)
{
return(distance(new Point(x, y)));
}
But I prefer the other way since (to me) manufacturing a 'Point' object just to pass it to 'distance' seems less clean than simply dereferencing 'mX' and 'mY' to pass to the first 'distance' method.
Finally, you were to implement 'toString' for your class and your code seemed fine:
public String toString()
{
return("[ x: " + mX + ", y: " + mY + "]");
}
The only possible change I would suggest would be to use 'String.format' to control the number of decimal places shown. I would think two or three decimal places would be sufficient instead of the 10+ that are output by default.
public String toString()
{
return(String.format("[ x: %.2f, y: %.2f ]", mX, mY));
}
Here is the final version of the code with the above changes applied:
public class Point
{
public Point(double x, double y)
{
mX = x;
mY = y;
}
public Point()
{
this(0.0, 0.0);
}
public static Point fromPolarCoordinates(double radius, double theta)
{
double x = radius * Math.cos(theta);
double y = radius * Math.sin(theta);
return(new Point(x, y));
}
public double getAngleDegrees()
{
return(Math.toDegrees(getAngleRadians()));
}
public double getAngleRadians()
{
return(Math.atan2(mY, mX));
}
public double distance(double x, double y)
{
double xDelta = (x - mX);
double yDelta = (y - mY);
return(Math.sqrt(xDelta * xDelta + yDelta * yDelta));
}
public double distance(Point other)
{
return(distance(other.mX, other.mY));
}
public String toString()
{
return(String.format("[ x: %.2f, y: %.2f ]", mX, mY));
}
public double getX() { return(mX); }
public double getY() { return(mY); }
public void setX(double value) { mX = value; }
public void setY(double value) { mY = value; }
private double mX;
private double mY;
public static void main(String[] args)
{
Point p1 = Point.fromPolarCoordinates(100, Math.toRadians(45.0));
Point p2 = new Point(3.0, 4.0);
Point p3 = new Point();
System.out.printf("p1: %s\n", p1.toString());
System.out.printf("p2: %s\n", p2.toString());
System.out.printf("p3: %s\n\n", p3.toString());
System.out.printf("p1, angle degrees: %.2f\n", p1.getAngleDegrees());
System.out.printf("p2, angle radians: %.2f\n\n", p2.getAngleRadians());
System.out.printf("distance, p2 -> p3: %.2f\n", p2.distance(p3));
p3.setX(12.0);
p3.setY(5.0);
System.out.printf("p3.x = %.2f, p3.y = %.2f\n", p3.getX(), p3.getY());
System.out.printf(
"distance, p3 -> origin: %.2f\n", p3.distance(0.0, 0.0)
);
}
}