Coding on Point interfaces/superclasses instead of implementations

Question

From pretty much any book I've read it always said to code on interfaces/superclasses and rarely on implementations, today though that advice made a bit confused about how I should format my code.

Questions:

1. Should I be using Point2DAdvanced or Point2D.Double in the the methods of MedianPoint (like JavaRDD<>, the 2 private methods, the variables)? Same goes for FlagPointPairProducer.
2. Should I be using MedianPoint or Point2D.Double in the FlagPointPairProducer class?
3. Should I store the MedianPoint from the static method in a MedianPoint or Point2D.Double?

The Code:

First off I have a MedianPoint class which extends Point2D.Double. This class is responsible for outputting a median point out of a collection of points (for now only an RDD).

public class MedianPoint extends Point2D.Double {
 public MedianPoint(double x, double y) {
 super(x, y);
 }
 
 public static MedianPoint fromPointRDD(JavaRDD<Point2DAdvanced> points) {
 Point2DAdvanced biggestPointByXDimension = points.reduce((a, b) -> getBiggestPointByXDimension(a, b));
 Point2DAdvanced biggestPointByYDimension = points.reduce((a, b) -> getBiggestPointByYDimension(a, b));
 double xDimensionMedian = biggestPointByXDimension.getX() / 2.0;
 double yDimensionMedian = biggestPointByYDimension.getY() / 2.0;
 return new MedianPoint(xDimensionMedian, yDimensionMedian);
 }
 private static Point2DAdvanced getBiggestPointByXDimension(Point2DAdvanced first, Point2DAdvanced second) {
 return first.getX() > second.getX() ? first : second;
 }
 private static Point2DAdvanced getBiggestPointByYDimension(Point2DAdvanced first, Point2DAdvanced second) {
 return first.getY() > second.getY() ? first : second;
 }
}

I also have a FlagPointPairProducer class which according to a median point it calculates a subspace flag for a given Point2DAdvanced (another Point2D.Double extension) and returns a tuple of the flag and the point.

public class FlagPointPairProducer implements Serializable {
 private final Point2D.Double medianPoint;
 public FlagPointPairProducer(Point2D.Double medianPoint) {
 this.medianPoint = medianPoint;
 }
 public Tuple2<PointFlag, Point2DAdvanced> getFlagPointPair(Point2DAdvanced point) {
 PointFlag flag = calculateFlag(point);
 return new Tuple2<>(flag, point);
 }
 private PointFlag calculateFlag(Point2DAdvanced point) {
 double x = point.getX();
 double y = point.getY();
 double medianX = medianPoint.getX();
 double medianY = medianPoint.getY();
 int xBit = x < medianX ? 0 : 1;
 int yBit = y < medianY ? 0 : 1;
 return new PointFlag(xBit, yBit);
 }
}

So I can initialize a FlagPointPairProducer like so:

Point2D.Double medianPoint = MedianPoint.fromPointRDD(points);
FlagPointPairProducer producer = new FlagPointPairProducer(medianPoint);

---

Code requested for Point2DAdvanced:

public class Point2DAdvanced extends Point2D.Double implements Serializable {
 public Point2DAdvanced(double x, double y) {
 super(x, y);
 }
 public boolean dominates(Point2D.Double point) {
 return (x <= point.getX() && y < point.getY())
 || (y <= point.getY() && x < point.getX());
 }
 @Override
 public String toString() {
 StringBuilder builder = new StringBuilder();
 builder.append('(').append(x).append(", ").append(y).append(')');
 return builder.toString();
 }
 public static Point2DAdvanced fromTextLine(String textLine, String delimiter) {
 textLine = textLine.trim();
 String[] lineArray = textLine.split(delimiter);
 double x = java.lang.Double.parseDouble(lineArray[0]);
 double y = java.lang.Double.parseDouble(lineArray[1]);
 return new Point2DAdvanced(x, y);
 }
}

• Gerold Broser: What's the point of having static fromPointRDD()? That's not what has been suggested to you in codereview.stackexchange.com/a/70878/58906 and codereview.stackexchange.com/a/70905/58906. You commented "This is mostly for project use so I guess you are right and I should extend it rather than wrap it both in this case and my other question." But you still have Point2Advanced (which is your former 'Point' I guess) the methods of which can be moved into MedianPoint.
• Gerold Broser: With the stacic fromPointRDD() deriving MedianPoint from Point2D.Double doesn't make sense. You could use a bare Point2D.Doubleinstead.
• Aki K: @GeroldBroser I have it as static so it can be consistent with Nihathrael's answer (I like the way he solved the super() issue). The methods of Point2DAdvanced can't be moved to MedianPoint, these two classes have an entirely different purpose. One cares about point domination issues and the other cares about being the median point of a collection. But if you have a suggestion feel free to write an answer.
• Gerold Broser: Can you supply the code of Point2DAdvanced as well?
• Aki K: @GeroldBroser sure (I edited it in).

Answer 1

Inheritance is not to be taken lightly. Have you considered composition instead?

The MedianPoint class seems to represent a point that happens to be median in some collection of points. But this depends on the context. There is nothing in this class that could prevent creating points that are not in fact at the median. You can certainly call the constructor or a factory method with parameters that don't correspond to a median point. As such, it would make more sense to use regular point objects, and distinguish them from non-median points by naming.

The methods in this class can be static methods in a PointUtils good old fashioned utility class.

The same goes for Point2DAdvanced as well. So the answer to all your questions: use Point2D.Double everywhere. The custom point classes don't really add value, and do nothing to help reducing complexity.

• Aki K: MedianPoint is more of a "approximate median point" from a collection sometimes. Meaning I added a constructor which hardcodes x and y because the computation of median is sometimes quite an overhead to the skyline algorithm itself and many times it is very close to (5, 5) so I left the option to hardcode it in order to measure efficiency later down the line.
• Aki K: As for having a static utility class in place of extending seems a bit of a downgrade to me? I don't know, I am not an awfully experienced developer so I am probably wrong on that but I have always been avoiding static utility classes... I am wrong on that? I can certainly see the advantage in this case (shoving both median function and dominates in a simple utility class) it just feels a bit "weird".
• Aki K: Also one question. Say I use Point2D.Double exclusively and scrap Point2DAdvanced. Should I then just use Point2D like so Point2D point = new Point2D.Double(x, y); for declaration since I only care about x and y?
• janos: It's best to program against the most general interfaces that make sense in your logic. If your code works fine by using Point2D in all type declarations and signatures without ever having to cast to Point2D.Double, then by all means use Point2D. Utility classes are not great, but perfectly acceptable for pure functions that don't depend on state, only depend on the method parameters.

Answer 2

OK, then no need for a Point2DAdvanced:

import java.awt.geom.Point2D;
/**
 * ...
 */
public class MedianPoint extends Point2D.Double {
 /**
 * ...
 */
 public static MedianPoint from( String twoDoubles, String limit ) {
 // no need to trim since Double.parseDouble(...) below trims anyway
 String[] doubles = twoDoubles.split( limit );
 // Note from Double.valueOf(...):
 // "To interpret localized string representations 
 // of a floating-point value, use subclasses of NumberFormat." 
 return new MedianPoint(
 java.lang.Double.parseDouble( doubles[0] ),
 java.lang.Double.parseDouble( doubles[1] )
 );
 }
 /**
 * ...
 */
 private static Point2D.Double greaterByXOf( Point2D.Double p1, Point2D.Double p2 ) {
 return p1.getX() > p2.getX() ? p1 : p2;
 }
 /**
 * ...
 */
 private static Point2D.Double greaterByYOf( Point2D.Double p1, Point2D.Double p2 ) {
 return p1.getY() > p2.getY() ? p1 : p2;
 }
 /**
 * ...
 */
 public MedianPoint( double x, double y ) {
 super( x, y );
 }
 /**
 * ...
 */
 public MedianPoint(JavaRDD<Point2D.Double> points) {
 super(
 points.reduce((a, b) -> greaterByXOf(a, b)).getX() / 2.0,
 points.reduce((a, b) -> greaterByYOf(a, b)).getY() / 2.0;
 );
 }
 /**
 * ...
 */
 public boolean dominates( Point2D.Double point ) {
 return x <= point.getX() && y < point.getY() ||
 y <= point.getY() && x < point.getX();
 }
 @Override
 public String toString() {
 return String.format( "(%f,%f)", x, y );
 }
 } // MedianPoint

Does this fit your needs?

If you're not happy with the class' name MedianPoint (because it is not such) rename it to something that suits you better.

• Aki K: I think it looks good this way as well.
• Aki K: Oh by the way the trimming is needed because if the string has any extra space the the doubles array will have 3 elements or more instead of two.
• Gerold Broser: You're right if you allow whitespace characters for the delimiter. But then you have the same problem if the string looks like "1.0␣␣2.0" and the delimiter is a space.

Answer 3

Some questions come in mind:

• Q: What behavior adds MedianPoint to Point2D.Double.

• A: None. The static methods do not, and there's nothing else.

• Q: How can I tell a MedianPoint from an ordinary Point2D.Double.

• A: Nothing but instanceof check. There's nothing special about it.

• Q: So why MedianPoint extends Point2D.Double?

• A: It's wrong. It should be a factory (method or class).

---

double xDimensionMedian = biggestPointByXDimension.getX() / 2.0;

"Half of the maximum" is surely not the definition of median I've been taught.

---

private final Point2D.Double medianPoint;

This seems to confirm the above. A medianPoint is just a Point2D.Double.

---

int xBit = x < medianX ? 0 : 1;
int yBit = y < medianY ? 0 : 1;
return new PointFlag(xBit, yBit);

Why not boolean?

---

Point2DAdvanced:

It's nice to add some functionality to a class, but it's not possible to do it for an object. So in order to be able to use dominates, you need to forget about the existing Point2D.Double and create a new object. IMHO too impractical.

This gets worsened by the fact that Point2D.Double is mutable. You may call dominates at a point when the original has changed. This may be what you want or not, but usually it's confusing at best. IMHO it was a mistake to make Point2D.Double mutable and I'd avoid it completely.

If you really want to use Point2DAdvanced as is, then you should probably provide a constructor or a factory taking a Point2D.Double. If you're only reusing part the Point2D.Double's functionality, then it's probably not worth it.

• Aki K: About median, it is more a median of each dimension stored in a point rather than the median of the whole space. Specifically I am using the D & C algorithm in order to partition the data in 2^d parts (where d is the number of dimensions therefore 4 parts in my case). The median of each dimension is the point of division. For more details you can see this paper on page 4, third paragraph delab.csd.auth.gr/~apostol/thesis1.pdf
• Aki K: As for PointFlag boolean would not represent really well what xBit and yBit represent (the fact that 0 means they are lower and 1 means they are higher). As for the rest you are right I will consider not using Point2D.Double.