8
\$\begingroup\$

I have a model of the solar system that I'm working on at the moment. Now is the time to start cleaning my code up a little.

Here's what I have so far:

A main 'driver' method:

public class SolarModel {

public static void main(String[] args){
    SolarSystem model = new SolarSystem(700, 700);

    /* Create new instances of each object in the solar system */
    SunObject sun = new SunObject(0, 0, 50, "YELLOW", 0);
    PlanetObject mercury = new PlanetObject(60, 30, 15, "LIGHT_GRAY" ,5);
    PlanetObject venus = new PlanetObject(90, 30, 15, "GREEN", 3);
    PlanetObject earth = new PlanetObject(140, 30, 15, "BLUE", 2);  
    PlanetObject mars = new PlanetObject(180, 30, 15, "RED", 1.2);

    MoonObject moon = new MoonObject(15, 30, 2, "WHITE", 1.5);

    while(true){
        /*Draw all the planets onto the SolarSystem model */
        model.drawSolarObject(sun.getDistance(), sun.getAngle(), sun.getDiameter(), sun.getColour());
        model.drawSolarObject(mercury.getDistance(), mercury.move(), mercury.getDiameter(), mercury.getColour());
        model.drawSolarObject(venus.getDistance(), venus.move(),venus.getDiameter(), venus.getColour());
        model.drawSolarObject(earth.getDistance(), earth.move(), earth.getDiameter(), earth.getColour());
        model.drawSolarObject(mars.getDistance(), mars.move(), mars.getDiameter(), mars.getColour());

        /* Draw the Earth's moon.
         * Use the Earth's PointInSpace as a point in which the moon orbits around      
         */
        model.drawSolarObjectAbout(moon.getDistance(),moon.getAngle() + moon.move(), moon.getDiameter(), moon.getColour(),
                                    earth.getDistance(), earth.getAngle());

        /* Draw Mars's moon.
         * Use Mars's PointInSpace as a point in which the moon orbits around       
         */
        model.drawSolarObjectAbout(moon.getDistance(), moon.getAngle() + moon.move(), moon.getDiameter(), moon.getColour(),
                                    mars.getDistance(), mars.getAngle());

        model.finishedDrawing();
        }
    }

}

A PointInSpace class:

public class PointInSpace {

private double distance;
private double angle;
private double diameter;
private String colour;
private double speed;

public double getDistance(){
    return distance;
}
public double getAngle(){
    return angle;
}
public double getDiameter(){
    return diameter;
}
public String getColour(){
    return colour;
}
public double getSpeed(){
    return speed;
}

public double move(){
    angle += speed;
    return angle;
}

public PointInSpace(double distance, double angle, double diameter, String colour, double speed){
    this.distance = distance;
    this.angle = angle;
    this.diameter = diameter;
    this.colour = colour;
    this.speed = speed;
    }

}

And classes representing Objects in the solar system

public SunObject(double distance, double angle, double diameter, String colour, double speed){
    super(distance, angle, diameter, colour, speed);

    }
}
    public PlanetObject(double distance, double angle, double diameter, String colour, double speed){
    super(distance, angle, diameter, colour, speed);

    }
}
    public MoonObject(double distance, double angle, double diameter, String colour, double speed){
    super(distance, angle, diameter, colour, speed);

    }
}

Any tips on how I can possibly refactor this code? As you can tell my 'driver' class is huge. How would I pass each object into an array and use the drawSolarObject() method from there? Any other possible way to cut down the size of this class?

What are the other weaknesses in my code?

\$\endgroup\$
5
\$\begingroup\$

First up, this is a nice project, and your implementation looks like it has the right basic concepts, but you're right, it could be done better.

There are two concepts you need to apply that will help you a lot...

Parent -> child relationships

The first is the concept of a parent/child relationship. You should consider that each object in your solar system has one or more child objects. The sun has the planets, the planets may have moons.

The logic of 'animating' your solar system is common to all objects. First, you move the object, then you move the dependent ones.

To do this, I extend the PointInSpace class to be:

public class ObjectInSpace extends PointInSpace {
    private final List<ObjectInSpace> dependents = new ArrayList<>();
    private final ObjectInSpace parent;

    public ObjectInSpace(ObjectInSpace parent, double distance, double angle, double diameter, String colour, double speed) {
        super(distance, angle, diameter, colour, speed);
        if (parent != null) {
            parent.addDependent(this);
        }
    }

    private void addDependent(ObjectInSpace kid) {
        // can only be called from constructor of kid instance
        dependents.add(kid);
    }

    public ObjectInSpace getParent() {
        return parent;
    }
    public List<ObjectInSpace> getDependents() {
        // return a copy, not the actual list.
        return new ArrayList<>(dependents);
    }
}

Right, now, with the above class, you can construct your solar system from the top down....

ObjectInSpace sun = new ObjectInSpace(null, .....);
....
ObjectInSpace earth = new ObjectInSpace(sun, ....);
....
ObjectInSpace moon = new ObjectInSpace(earth, ....);

Note that the only thing you need to remember is the sun, and all the other objects 'hang off' the sun. So, create a method:

public ObjectInSpace buildSolarSystem() {
    ObjectInSpace sun = .....
    ....
    return sun;
}

and then, in your code, you can just have:

ObjectInSpace solarsystem = buildSolarSystem();

Now, when you need to draw/move the solar system, you just need to visit every object, from the sun down, and process that object, then process in turn all of its children...

... which leads on to the second concept:

Recursion

The next concept to process is recursion. Consider the need to draw the solar system. All you need is a method:

public void draw(ObjectInSpace object) {
    // draw the object
    ....
    for (ObjectInSpace kid : object.getDependents()) {
        // recurse and draw the kid
        draw(kid);
    }
}

Something similar can be done to move the objects:

public void move(ObjectInSpace object) {
    // move the object
    if (object.getParent() != null) {
        // move the object relative to the parent.
    }
    ....
    for (ObjectInSpace kid : object.getDependents()) {
        // recurse and move the kid
        move(kid);
    }
}
\$\endgroup\$
  • \$\begingroup\$ As a side note: It's typically called the Composite design pattern. \$\endgroup\$ – ChrisWue Oct 25 '14 at 17:34

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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