-4
\$\begingroup\$

The task is to model a Solar system using OOP. The planets are orbiting and its needs to find distances between planets.

My approach is something like this.

public abstract class SolarSystem {
    // TODO
}

public class Planet extends SolarSystem {

    private final String name;
    private final double x;
    private final double y;

    public Planet(String name, double x, double y) {
        this.name = name;
        this.x = x;
        this.y = y;
    }

    public double distanceTo(Planet p) {
        return Math.sqrt((p.x - this.x) * (p.x - this.x) + (p.y - this.y) * (p.y - this.y));
    }

}

public class Main {

    public static void main(String[] args) {
        Planet earth = new Planet("Earth", 5.2, 8.3);
        Planet mars = new Planet("Mars", 4.3, 2.7);

        System.out.println(mars.distanceTo(earth));
    }
}

I am stuck with the question of how to design Planet class in a way that it is open for changes?

For example, let's say that the requirements change a bit and two-dimensional space changes to three-dimensional space. At the moment, my approach supports only one way to calculate distance.

What am I missing here?

\$\endgroup\$
12
  • 3
    \$\begingroup\$ Welcome to the Code Review Community. We only review code that is working as expected, there are other sites that will help you debug your code. Please read Where can I get help? and How do I ask a good question?. \$\endgroup\$
    – pacmaninbw
    Feb 4 at 14:06
  • 4
    \$\begingroup\$ Software engineering might be a better place for a design question. The // TODO makes it off-topic and the code isn't working yet. You are probably going to need a z axis component. \$\endgroup\$
    – pacmaninbw
    Feb 4 at 14:28
  • 2
    \$\begingroup\$ Is planet-to-planet distance any different then a point-to-point distance? I mean why not Planet<TCoordinate> and let TCoordinate be 2D, 3D or whatever coordinate you want as long as the coordinate type can tell distance to another coordinate of the same type. \$\endgroup\$
    – slepic
    Feb 4 at 14:35
  • 3
    \$\begingroup\$ (A planet extends a solar system the way an element extends a collection.) \$\endgroup\$
    – greybeard
    Feb 4 at 15:17
  • 3
    \$\begingroup\$ Please (re)visit Which computer science/programming Stack Exchange sites do I post on? \$\endgroup\$
    – greybeard
    Feb 4 at 15:20

2 Answers 2

3
\$\begingroup\$

I am sorry that you recieved so negative feedback in the comments. It is true that your code isn't actually modelling any dynamic system which from your description seems to be your goal.

However, your code is working, and can be deemed as first stage prototype. Maybe you are stuck or want to perfectionate your stage one before going to another stage. And I think it is fair to review your working code in this stage, even if it's not supposed to be final.

So to the review!

There really is no reason why a Planet should extend SolarSystem. If you cannot justify statement "A is B" then never model relation between A and B with inheritance.

I would much rather understand inheritance relation "Planet extends CelestialBody" because a planet is a celestial body.

But anyway in your current code that abstraction is useless. An abstract class or interface with no methods or properties is never of any use.

Now to your concern how to make your class open to be used with any kind of coordinates. You can achieve that with a generic type.

Let me show you how.

First let me first define a separate class for the coordinates x,y. Later you can create a new class for 3D coordinates.

public class Point2D {
    public final double x;
    public final double y;

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

    public double getDistanceSquare(Point2D other) {
        double dx = other.x - this.x;
        double dy = other.y - this.y;
        return dx * dx + dy * dy;
    }

    public double getDistance(Point2D other) {
       return Math.sqrt(this.getDistanceSquare(other));
    }
}

Notice that I extracted getDisanceSquare() method separately because it might come handy when you start dealing with for example Newton's law of gravity. Gravitation force/potential is proportional to distance square and it would be ineffective to compute the square root only to then square the result.

I will rename the Planet class to Pin to just stand for a position with a name, to not imply that it is anything more than that at this point.

public class Pin<TCoordinate> {
    public final String name;
    public final TCoordinate position;

    public Pin(String name, TCoordinate position) {
        this.name = name;
        this.position = position;
    }
}

public class Main {

    public static void main(String[] args) {
        Pin<Point2D> earth = new Pin<Point2D>("Earth", new Point2D(5.2, 8.3));
        Pin<Point2D> mars = new Pin<Point2D>("Mars", new Point2D(4.3, 2.7));

        System.out.println(mars.position.getDistance(earth.position));
    }
}

Notice that the Pin now does not have it's own distance method. Just use disantce of the positions.

Now when you create a 3D coordinate class. You can just instantiate Pin to create a 3D model of similar kind. And all your code will still work as long as Point3D also has a double getDistance(Point3D other) method.

But remember Pin<Point2D> is not compatible with Pin<Point3D>. They live in separate universes. One lives in a 2 dimensional universe and the other in 3 dimensional one.

One last note, your use of final is good. It makes your classes immutable which is good. But notice, in my implementation i made them public, because eventually you may want to access those values maybe for serialization or for display purpose. Normally you would add getters, but with final props, making them public is just as fine.

PS: The code is not tested and I'm not even a Javist. I hope it's ok but sorry if I messed up somewhere.

\$\endgroup\$
4
  • \$\begingroup\$ Must disagree about interface with no methods or properties is never of any use. There are marker interfaces like Serializable, Cloneable and some other. So in general it´s not true, but in this case I agree. \$\endgroup\$
    – convert
    Feb 5 at 21:56
  • \$\begingroup\$ @convert marker interfaces are code smell. A serializable interface should provide a method that returns the serialized representation of whatever. Clonable interface should have a method that return the new clone. I have no idea how these examples can be just marker interfaces and no code smell. \$\endgroup\$
    – slepic
    Feb 6 at 7:03
  • \$\begingroup\$ Sorry, looks like a misunderstanding, I absolutly agree with you that this interface is of no use. \$\endgroup\$
    – convert
    Feb 6 at 11:27
  • \$\begingroup\$ Super nice! Thank you! \$\endgroup\$ Feb 7 at 10:01
0
\$\begingroup\$

What that SolarSystem abstract class should be good for? Also by:

public class Planet extends SolarSystem

you saing that a planet is solar system which is not true. Do x and y stand for x and y position of the planet? What about z position then? Also it seems then to be some very strange static solar system, where all planets stand still. You could create a Position class to encapsule x and y values, or even Position interface so you can have diferent types of positions like 2D, 3D, etc..

So the interface example could look like this:

    public interface Position {
        double getX();
        double getY();
        double getZ();
        double distance(Position other);
    }
    
    public class Position2D implements Position{
        private double x;
        private double y;
        
        public double getX(){
            return x;
        }
        
        public double getY(){
            return y;
        }
        
        public double getZ(){
            return 0;
        }
        
       public  double distance(Position other){
           double dx=other.getX()-x;
           double dy=other.getY()-y;
           double dz=other.getZ();
           return return Math.sqrt(dx*dx+dy*dy+dz*dz);
       }
    }

public class Planet {

    private final String name;
    private Position position;

    public Planet(String name, Position position) {
        this.name = name;
        this.position = position;
    }

    public double distanceTo(Planet p) {
        return position.distance(p.position);
    }

}

An abstract class could posibly be even a beter solution. Also the 2D position in this example can be changed, this is the reason I am not using final for x and y.

\$\endgroup\$
9
  • 5
    \$\begingroup\$ Please refrain from answering questions that are off-topic. This would be better as a comment. \$\endgroup\$
    – pacmaninbw
    Feb 4 at 16:33
  • \$\begingroup\$ @pacmaninbw The question defenetly need to be edited, but then it can be on topic. \$\endgroup\$
    – convert
    Feb 4 at 17:17
  • 4
    \$\begingroup\$ Code Review rules prohibit the original poster from editing the code once an answer has been posted, because retroactive edits make a mess of the review system. Please refrain from answering questions that are off-topic. \$\endgroup\$ Feb 5 at 6:22
  • \$\begingroup\$ @convert can you show an example of that Position interface? I think that is the right direction. Or do you have any source you can point me? \$\endgroup\$ Feb 5 at 7:45
  • \$\begingroup\$ @AverageJoe9000 Added an example for interface, but it´s not perfect. \$\endgroup\$
    – convert
    Feb 5 at 11:34

Not the answer you're looking for? Browse other questions tagged or ask your own question.