Solar System model

Question

I have a model that works, as far as I know, but it's so messy! I am very new to Java, so I'd really appreciate some help tidying up. In particular, a lot of my constructors are empty which is probably not good, and I'm not sure if I've made the right choices in terms of public, static, void etc for my methods. It probably doesn't follow best practices.

SolarSim (Main):

import java.util.Scanner;
import java.lang.Math;
import java.util.Arrays;
import java.io.*;
public class SolarSim{

    private static final double earthMass = 5.9726*Math.pow(10,24);
    private static final double earthRadius = 6371000;
    private static final double sunMass = 1.9885*Math.pow(10,30);
    private static final double sunRadius = 696342000;
    private static final double mercuryMass = 3.301*Math.pow(10,23);
    private static final double mercuryRadius = 2.44*Math.pow(10,6);
    private static final PhysicsVector zero = new PhysicsVector(0,0);
    private static final PhysicsVector zero1 = new PhysicsVector(0,0);
    private static final PhysicsVector zero2 = new PhysicsVector(0,0);
    private static final PhysicsVector zeroa = new PhysicsVector(0,0);
    private static final PhysicsVector zerob = new PhysicsVector(0,0);
    private static final PhysicsVector zeroc = new PhysicsVector(0,0);

    public static PhysicsVector[] copyArray(PhysicsVector[] a) {
        int length = a.length;
        PhysicsVector[] copy = new PhysicsVector[length];
        System.arraycopy(a, 0, copy, 0, length);
        return copy;
    }

    public static double sumArray(double[] p){
        double sum = 0;
        for(int z= 0; z < p.length; z++){
            sum += p[z];
        }
        return sum;
    }

    public static PhysicsVector[] add(PhysicsVector[] sum, PhysicsVector a){
        for(int c = 0; c<sum.length; c++){
            sum[c].increaseBy(a);
        }
        return sum;
    }

    public static PhysicsVector[] subtract(PhysicsVector[] diff, PhysicsVector g){
        for (int ab=0; ab<diff.length;ab++){ 
            diff[ab].decreaseBy(g);
        }
        return diff;
    }   
    public static void main(String[] args) throws IOException{

        java.io.File file = new java.io.File("output.txt" );
        java.io.PrintWriter n = new PrintWriter(file);

        //Initialise variables here
        PhysicsVector earthInitialPos = new PhysicsVector();
        PhysicsVector earthInitialV = new PhysicsVector();
        PhysicsVector sunInitialV = new PhysicsVector();
        PhysicsVector sunInitialPos = new PhysicsVector();
        PhysicsVector mercuryInitialPos = new PhysicsVector();
        PhysicsVector mercuryInitialV=new PhysicsVector();

        Scanner scanner = new Scanner(System.in);       
        System.out.println("Please enter the size of the time step:");
        double timeStep = scanner.nextDouble();

        //SET PLANETS' INITIAL POSITIONS
        earthInitialPos.setVector(1.4960*Math.pow(10,11),0);
        earthInitialV.setVector(0,29786.24);

        sunInitialPos.setVector(0,0);
        sunInitialV.setVector(0,0);

        mercuryInitialPos.setVector(5.791*Math.pow(10,10),0);
        mercuryInitialV.setVector(0,47873.5);

        //CREATE GRAVFIELD OBJECTS
        GravField sunGravField = new GravField(sunMass, sunRadius, sunInitialPos);
        GravField earthGravField = new GravField(earthMass, earthRadius, earthInitialPos);
        GravField mercuryGravField = new GravField(mercuryMass, mercuryRadius, mercuryInitialPos);

        //CREATE PARTICLE OBJECTS   
        Particle earth = new Particle(earthMass, earthInitialPos, earthInitialV);
        Particle sun = new Particle(sunMass, sunInitialPos, sunInitialV); 
        Particle mercury = new Particle(mercuryMass, mercuryInitialPos, mercuryInitialV);

        double time = 0;
        double finalTime = 31557600;            //One earth year(seconds)

        PhysicsVector newSunGrav = new PhysicsVector();
        PhysicsVector newEarthGrav = new PhysicsVector();
        PhysicsVector newMoonGrav = new PhysicsVector();

        GravField[] gravityObject = {earthGravField, sunGravField, mercuryGravField};
        PhysicsVector[] position = {earthInitialPos, sunInitialPos, mercuryInitialPos}; 
        PhysicsVector[] velocity = {earthInitialV, sunInitialV, mercuryInitialV};
        PhysicsVector[] gravField = {zero, zero1, zero2};
        double[] planetMass = {earthMass, sunMass, mercuryMass};
        Particle[] planets = {earth, sun, mercury};

        //Calculate the centre of mass and subtract position from positions of planets, so c.o.m is at origin
        PhysicsVector centreOfMass = new PhysicsVector();
        centreOfMass = sun.centreOfMass(planetMass, position);
        position = SolarSim.subtract(position, centreOfMass);
        System.out.println(Arrays.toString(position));
        centreOfMass.print();

        //Calculate centre of mass velocity and subtract from planet velocities
        PhysicsVector centreOfMassVelocity = new PhysicsVector();
        centreOfMassVelocity = sun.cOMVel(planetMass, velocity);
        centreOfMassVelocity.print();
        velocity = SolarSim.subtract(velocity, centreOfMassVelocity);

        //Calculate fields of planets
        for(int ac=0; ac<gravityObject.length; ac++){
            for(int ad=0;ad<gravityObject.length;ad++){
                if(ac!=ad){
                    gravField[ac].increaseBy(gravityObject[ac].aDueToGravity(planetMass[ad], position[ad], position[ac]));
                }
                else{
                    //do nothing
                }
            }
        }
        PhysicsVector[] newP = new PhysicsVector[position.length];
        PhysicsVector[] newGrav = {zeroa,zerob,zeroc};
        PhysicsVector[] newVel = new PhysicsVector[velocity.length];

        do{
            PhysicsVector[] y = new PhysicsVector[gravField.length];
            y=copyArray(gravField);

            for(int i=0; i<planets.length;i++){
                newP[i] = planets[i].updatePosition(position[i], velocity[i], timeStep, gravField[i]);
            }

            for(int j=0; j<gravityObject.length; j++){    
                for(int l=0;l<gravityObject.length;l++){
                    if(j!=l){
                        newGrav[j].increaseBy(gravityObject[j].aDueToGravity(planetMass[l], newP[l], newP[j]));
                    }
                    else{
                        //do nothing
                    }
                }
            }

            for(int k=0; k<planets.length; k++){
                newVel[k] = planets[k].updateVelocity(velocity[k], timeStep, y[k], newGrav[k]);
            }

            //Calculate centre of mass velocity and subtract from planet velocities
            centreOfMassVelocity = earth.cOMVel(planetMass, newVel);
            for (int ab=0; ab<newVel.length;ab++){ 
                newVel[ab].decreaseBy(centreOfMassVelocity);    
            }
            newVel = SolarSim.subtract(newVel, centreOfMassVelocity);

            gravField = copyArray(newGrav);
            velocity = newVel;
            position = newP;

            time+=timeStep;
            double x = newP[0].getX();
            double ap = newP[0].getY();
            n.println(x+"   "+ap);

        }while (time<=1000*finalTime);
        System.out.println(Arrays.toString(newP));


        n.close();
    }

}

Particle:

import java.lang.Math;

//Class that creates a particle object with mass, initial velocity and initial position
public class Particle{

    private double mass;
    PhysicsVector initialPosition = new PhysicsVector();
    PhysicsVector initialVelocity = new PhysicsVector();
    PhysicsVector centreOfMass = new PhysicsVector(0,0);
    PhysicsVector cOMV=new PhysicsVector(0,0);

    //Default constructor
    public Particle(){
        mass = 1;
        initialPosition.setVector(0,0);
        initialVelocity.setVector(1,1);
    }

    //Constructor 
    public Particle(double mass, PhysicsVector x, PhysicsVector y){

    }


    //Make it static or not? 
    public PhysicsVector updatePosition(PhysicsVector initialPosition, PhysicsVector initialVelocity, double timeStep, PhysicsVector aDueToGravity){

        PhysicsVector x = new PhysicsVector(initialVelocity);
        x.scale(timeStep);

        PhysicsVector z = new PhysicsVector(aDueToGravity);
        z.scale(0.5*timeStep*timeStep);

        initialPosition.increaseBy(x);
        initialPosition.increaseBy(z);
        return initialPosition;

    }
    public PhysicsVector updateVelocity(PhysicsVector initialVelocity, double timeStep, PhysicsVector a, PhysicsVector newA){
        PhysicsVector z = new PhysicsVector(newA);
        PhysicsVector x = new PhysicsVector(a);
        z.increaseBy(x);
        z.scale(0.5*timeStep);
        initialVelocity.increaseBy(z);
        return initialVelocity;
    }


    public PhysicsVector centreOfMass(double[] mass, PhysicsVector[] positions){
        //Set origin at centre of sun, so that sunMass*distance = 0
        double sum = SolarSim.sumArray(mass);
        for(int i=0;i<positions.length;i++){
            centreOfMass.increaseBy(positions[i].scale(mass[i],positions[i]));
        }
        centreOfMass.scale(1/sum);

        return centreOfMass;
    }

    public PhysicsVector cOMVel(double[] mass, PhysicsVector[] velocity){
        double total = SolarSim.sumArray(mass);
        for(int ae=0;ae<velocity.length;ae++){
            cOMV.increaseBy(velocity[ae].scale(mass[ae],velocity[ae]));
        }
        cOMV.scale(1/total);
        return cOMV;
    }
}

GravField:

import java.lang.Math;
import java.io.*;


//Class to create a gravity object.

public class GravField{

    public static final double G = 6.67408*Math.pow(10,-11);            

    private double planetMass;                      
    private double planetRadius;                        
    PhysicsVector gravityAcceleration = new PhysicsVector();


    /**
    *Default constructor that creates a GravField object with the mass and radius of the earth,
    *acting on a projectile starting at x=0, y=0, where the x and y axes are on the surface of the planet
    */
    public GravField(){

    }

    /** 
    *Constructor that creates a GravField object
    *@param planetMass Mass of the planet whose field is to be calculated
    *@param planetRadius Radius of the planet
    */ 
    public GravField(double mass, double radius, PhysicsVector initialPos){
    //can't think of anything to do here
    }

    //Calculates the acceleration due to the gravitational field of the object
    public PhysicsVector aDueToGravity(double planetMass, PhysicsVector sourcePos, PhysicsVector initialPosition){

        PhysicsVector a = new PhysicsVector(sourcePos);
        PhysicsVector b = new PhysicsVector(initialPosition);

        b.decreaseBy(sourcePos);

        double distance = b.magnitude();
        b.scale(-1*G*planetMass/(distance*distance*distance));
        return b;
    }
}

Answer 1

After reading the program, I think it is pretty good for start, but there is some room for further improvement.

Small improvements

Imports

Consider importing specific classes, instead of using the wildcard import, so that your namespace is not cluttered up. (Although there are also benefits in importing the whole package, see this SO question).

Double constants

Instead of using Math.pow, like this:

5.9726*Math.pow(10,24)

Numbers can be written in scientific notation, as follows:

5.9726e24

Empty else blocks

In general, it is best to avoid empty else blocks, like this one:

if(j!=l){
    newGrav[j].increaseBy(gravityObject[j].aDueToGravity(planetMass[l], newP[l], newP[j]));
}
else{
     //do nothing
}

Instead, the else-block can just be omitted:

if(j!=l){
    newGrav[j].increaseBy(gravityObject[j].aDueToGravity(planetMass[l], newP[l], newP[j]));
}

JavaDoc

The parameters in the documentation should match the parameters of the method.

*@param planetMass Mass of the planet whose field is to be calculated
*@param planetRadius Radius of the planet
*/ 
public GravField(double mass, double radius, PhysicsVector initialPos){

So, in the above case, you should write "mass" and "radius" into the javadoc, instead of planetMass and planetRadius. Also, it would be nice to describe what "initialPos" means.

Visibility (public, protected, default, private) and staticness

Particle.initialPosition, Particle.initialVelocity, Particle.centreOfMass and Particle.cOMV are used only within Particle, thus they can be made private, instead of default (package-private) access.

The methods updatePosition, updateVelocity, centreOfMass and cOMVel cannot be made static, since they refer to the above mentioned member variables. (The other possibility would be to make those variables static as well, though I'm not sure if that would not break the logic of the program.)

Constructors

Empty body for constructors

As you wrote yourself, this is not a good practice :) In short, those constructors do almost nothing.

By calling public Particle(double mass, PhysicsVector x, PhysicsVector y), Particle.mass, Particle.initialPosition and Particle.initialVelocity are not set (they have the same value they received during initialisation). Probably, you should do something like this in the constructor:

public Particle(double mass, PhysicsVector x, PhysicsVector y){
     this.mass = mass;
     initialPosition = x;
     initialVelocity = y;
}

Although, it is not clear to me, what x and y mean in this context, so the actual code needed in your application could be different. Also note, that you need to prefix "mass" with "this", if you are accessing the member variable, to differentiate it from the constructor parameter with the same name.

The constructor public GravField(double mass, double radius, PhysicsVector initialPos) also does not do anything more than the default constructor, as it is now. Probably, it should look similar as follows:

public GravField(double mass, double radius, PhysicsVector initialPos){
     planetMass = mass;
     planetRadius = radius;
     // ? = initialPos;  // I have no idea for what initialPos should be used.
}

Note, that currently both constructors are being used, but they end up constructing three similar objects in each case (i.e., all with the default mass, radius, etc., instead of the parameters that you provide to them).

Default constructor

If you are not planning to use the default constructor (i.e. the one without any parameters), you do not need to provide one for the class. I.e., the following constructors are not really needed and can be removed:

public Particle(){
    mass = 1;
    initialPosition.setVector(0,0);
    initialVelocity.setVector(1,1);
}

public GravField(){

}

Separation of the code

Utility methods

I would suggest to move the methods SolarSim.copyArray, SolarSim.sumArray, SolarSim.add and SolarSim.subtract to a separate class (e.g. a new class called PhysicsVectorUtils, or even PhysicsVector itself, if you have access to its code), because they do not really belong to the logic of SolarSim. Also, in this way, Particle does not need to depend on SolarSim, in order to call sumArray.

main method

This method is very long, and difficult to follow. I suggest splitting up the steps into smaller methods. Besides, the local variables of this method, could be instead instance variable of SolarSim. You should end up with something like this:

public static void main(String[] args) throws IOException{
    SolarSim solarSim = new SolarSim();

    solarSim.openFile();
    solarSim.initVariables();
    solarSim.readTimeStep();
    solarSim.setInitialPositions();
    solarSim.createGravFieldObjects();
    solarSim.createParticleObjects();
    solarSim.initializeTimeAndGrvity();
    solarSim.calculateCentreOfMass();
    solarSim.calculateCentreOfMassVelocity();
    solarSim.calculateFields();

    do{
        solarSim.updateVeolcitiesAndPositions();
    }while (solarSim.hasMoreSteps());

    solarSim.printResult();
    solarSim.closeFile();
}

You could take the first part further, and call the methods not dependent on user input (file opening, initialisation of variables, fields, velocities etc.) from the constructor of SolarSim. (Beware, I'm not suggesting to put all the initialisation code into the constructor, because in this way the constructor would become very long. Just call those methods from within the constructor.)

Ideas for further improvements

The output file could be an argument of the program. I.e., you would invoke the program like this:

java SolarSim /path/to/my/customoutput.txt

The path to the file can be read from args[0] in this case (i.e., from the args parameter of main).

Also the number of steps (which is currently constant 1000), could be a parameter of the simulation.

Answer 2

What Attilio says is very correct, but I did notice some more ways to improve.

Built-in functions

• If you're using Java 8, you can use streams to simplify your sumArray method. Replace the body with return Arrays.stream(p).sum();.

• The method copyArray can be replaced with a call to .clone() for the same effect.