2
\$\begingroup\$

My program is meant to simulate 2D physics(hopefully 3D as well in the future), namely classical mechanics and eventually electromagnetism. Currently it simulates projectile motion by firing a cannonball(which is plainly a black ellipse) from an invisible cannon. It simply spawns an ellipse with an initial velocity and draws its motion.

My concern is that I have designed the program poorly in terms of organization and refactorability, if the latter is a word. Basically, I want to be able to build upon this model and add more simulations to it, so I want it to be designed well.

I used a GUI Form in IntelliJ IDEA to layout the JPanels, JLabels, etc.

If I want to eventually add collisions, explosions, magnetic fields, etc etc(perhaps in a tabbed pane that holds each type of simulation), am I moving in the correct direction in terms of design?

Edit: Any opinions or tips would answer this question.

Here is a screenshot(balls spawned in bottom-left corner):enter image description here

Here is my code:

ProjectileMotionApp.java

import javax.swing.*;
import java.util.Timer;
import java.util.TimerTask;

/*
 *  Creates JFrame and runs the simulation
 */
public class ProjectileMotionApp {

    public static void main(String[] args) {

        // The main panel that has embedded panels for the UI and the scene
        GUI gui = new GUI();

        // 60 fps (assuming computer can keep up)
        int DELAY = 1000 / 60;

        Timer timer;

        JFrame frame = new JFrame("Projectile Motion");
        frame.setContentPane(gui.getRootPanel());
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        frame.pack();
        frame.setSize(900, 600);
        frame.setVisible(true);

        timer = new java.util.Timer();

        timer.schedule(new TimerTask() {

            @Override
            public void run() {

                // has moving objects
                if(gui.getScene().inFlux()) {
                    gui.getScene().updateBalls();
                    gui.getScene().repaint();
                }
            }
        }, 0, DELAY);
    }
}

GUI.java

import javax.swing.*;
import java.awt.*;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;

/*
 *  Creates GUI.  Adds a UI and "scene"
 */
public class GUI {

    private JPanel rootPanel;
    private JPanel userInputPanel;

    private JButton FIREButton;
    private JTextField ballMassTextField;
    private JTextField ballFireVelocityTextField;
    private JTextField gravTextField;
    private JTextField airResistanceTextField;
    private JLabel ballMassLabel;
    private JLabel ballFireVelLabel;
    private JLabel gravityConstantLabel;
    private JLabel airResistanceLabel;
    private JLabel massUnitLabel;
    private JLabel gravUnitLabel;
    private JLabel velUnitLabel;
    private JLabel forceUnitLabel;
    private JTextField dirTextField;

    private Scene scene;

    // convert to seconds
    private final double deltaTime = 1 / (double)60;

    public GUI() {

        rootPanel = new JPanel();

        // tried BorderLayout to make scene as big as possible in JFrame, didn't work
        rootPanel.setLayout(new GridLayout(2, 1));

        // Instantiates scene with Earth-like gravity(9.8 m/s^2) and zero air resistance(0 N)
        scene = new Scene(9.8, 0, deltaTime);

        rootPanel.add(userInputPanel);
        rootPanel.add(scene);

        FIREButton.addActionListener(new ActionListener() {
            @Override
            public void actionPerformed(ActionEvent e) {

                //15 kg
                double mass = 15;
                // 150 m/s
                double velocity = 150;
                // 45 degrees
                double dir = 45;

                /* Replace default values */
                if(!ballMassTextField.getText().equals("")) {
                    mass = Double.parseDouble(ballMassTextField.getText());
                }
                if(!ballFireVelocityTextField.getText().equals("")) {
                    velocity = Double.parseDouble(ballFireVelocityTextField.getText());
                }
                if(!gravTextField.getText().equals("")) {
                    scene.setGravity(Double.parseDouble(gravTextField.getText()));
                }
                else {
                    scene.setGravity(9.8);
                }
                if(!airResistanceTextField.getText().equals("")) {
                    scene.setDrag(Double.parseDouble(airResistanceTextField.getText()));
                }
                else {
                    scene.setDrag(0);
                }
                if(!dirTextField.getText().equals("")) {
                    dir = Double.parseDouble(dirTextField.getText());
                }

                // makes angle with horizontal axis
                dir = Math.toRadians(360 - dir);

                Projectile ball = new Projectile(10, scene.getHeight() - (scene.getHeight() / 16), mass, velocity, dir);

                scene.addBall(ball);
            }
        });
    }

    public JPanel getRootPanel() {

        return rootPanel;
    }

    public Scene getScene() {

        return scene;
    }
}

Scene.java

import javax.swing.*;
import java.awt.*;
import java.awt.geom.Ellipse2D;
import java.util.ArrayList;

/*
 *  The "scene" of the simulation.  JPanel that draws balls
 */
public class Scene extends JPanel {

    private ArrayList<Projectile> balls;
    private ArrayList<Projectile> explosionBits;
    private ArrayList<Integer> locs;
    private double g;
    private double d;
    private double dt;

    public Scene(double gravity, double airResistance, double deltaTime) {

        this.setBackground(Color.WHITE);
        this.setSize(500, 800);

        balls = new ArrayList<>();
        explosionBits = new ArrayList<>();
        locs = new ArrayList<>();

        g = gravity;
        d = airResistance;
        dt = deltaTime;
    }

    public void addBall(Projectile ball) {

        balls.add(ball);
    }

    // adds exploded bits to scene on impact
    public void addExplosiveBits() {


    }

    /*
     *  Updates coordinates according to kinematic equations
     */
    public void updateBalls() {

        for(int i = 0; i < balls.size(); i++) {

            balls.get(i).updatePosition(dt, g, d);
            balls.get(i).updateVelocity(dt, g, d);

            if(balls.get(i).getXPos() < 0 || balls.get(i).getYPos() > this.getHeight()) {
                balls.remove(i);
                //addExplosiveBits();
                i--;
            }
        }
    }

    public void setGravity(double gravity) {

        g = gravity;
    }

    public void setDrag(double airResistance) {

        d = airResistance;
    }

    public ArrayList<Projectile> getBalls() {

        return balls;
    }

    public boolean inFlux() {

        if(balls.isEmpty())
            return false;

        return true;
    }

    public void paintComponent(Graphics g) {

        super.paintComponent(g);

        /* Cast it to Graphics2D */
        Graphics2D g2 = (Graphics2D) g;

        g2.setColor(Color.BLACK);

    /* Enable anti-aliasing and pure stroke */
        g2.setRenderingHint(RenderingHints.KEY_ANTIALIASING, RenderingHints.VALUE_ANTIALIAS_ON);
        g2.setRenderingHint(RenderingHints.KEY_STROKE_CONTROL, RenderingHints.VALUE_STROKE_PURE);


        for(Projectile ball : balls) {
            /* Construct a shape and draw it */
            Ellipse2D.Double shape = new Ellipse2D.Double(ball.getXPos(), ball.getYPos(), 5, 5);
            g2.fill(shape);
        }
    }
}

Projectile.java

/*
 *  Projectile with properties
 */
public class Projectile {

    private double xPos; // x-coordinate
    private double yPos; // y-coordinate
    private double xVel; // x component velocity magnitude
    private double yVel; // y component velocity magnitude
    private double dir;  // angle ball is shot from
                         // used to instantiate component velocities
    private double m;    // mass

    public Projectile(double x, double y, double mass, double velocity, double direction) {

        xPos = x;
        yPos = y;

        m = mass;

        dir = direction;

        xVel = velocity * Math.cos(dir);
        yVel = velocity * Math.sin(dir);
    }

    /*
     *  Updates coordinates according Kinematic Equations(metric system)
     *
     *  x = xo + vo t + ½ a t2
     */
    void updatePosition(double dt, double g, double d) {

        xPos = xPos + (xVel * dt) - ((d/m) * Math.pow(dt, 2) * 0.5);
        yPos = yPos + (yVel * dt) - (g * Math.pow(dt, 2) * 0.5);
    }

    /*
     *  Updates component velocities according Kinematic Equations(metric system)
     *
     *  v = vo + at
     */
    void updateVelocity(double dt, double g, double d) {

        xVel = xVel - dt * d;
        yVel = yVel + dt * g;
    }

    public double getXPos() {

        return xPos;
    }

    public double getYPos() {

        return yPos;
    }
}
\$\endgroup\$
  • \$\begingroup\$ In your Projectile class, when you simulate things that are not spheres you will need to also track rotational inertia and rotational velocity. It may be useful to also track the number of forces on an object too. \$\endgroup\$ – markspace May 17 '17 at 18:23
  • \$\begingroup\$ Also see here, not a great book on physics but a good introduction: shop.oreilly.com/product/0636920012221.do \$\endgroup\$ – markspace May 17 '17 at 18:27
2
\$\begingroup\$

My two cents:

Swing

Swing's dead, just wanted to mention that. I still use it too sometimes, to test stuff out.

GUI

The main issue I have with your code: I don't see a GUI pattern involved. You might want to check Martin Fowler's website, he has quite the list of Presentation Patterns with good examples. I mention that, beside the Single Responsibility Principle problem, because of the following reasons:

  • It's tightly coupled to Swing.
  • Presentation Logic is not separated. This can be a major pain in the future. If you have a large application and want to switch presentation technology and/or pattern, you'll have a very hard time, because the logic is not separated, therefore not reusable. For instance, in Scene.updateBalls: The Scene itself holds the responsibility to update a position. Or in ProjectileMotionApp, the update/calculate-process is scheduled and triggered within the main App - it updates the balls in the GUI and calls the repaint, but, a GameTicker (I'm just gonna call it like that) shouldn't be aware of a GUI, nor a Scene.

I think it's too much to ask, to point everything out or give more or less exact statements about what you may need to refactor to what and why, since with some reading about the presentation patterns, you should come up with your own pattern decisions which you then will implement. I think that needs a few iterations ;-)

(Check Passive View, MVP, MVVM/Presentation Model, that's the most common used, imo, or check for open source projects).

Comments

99% percent of your comments are obsolete. First, they explain what the code is doing - don't comment what you are doing. They will lie one day, for instances the // 60 fps comment. One day it will be 30, or dynamic, or whatever. If you comment, comment why you are doing something. But in general, even that is often not necessary. In enterprise applications, it's usual because of "business illogic" / absurd requirements.

Naming

Try to be as clear as possible when it comes to naming. For instance, in Scene, you declare g, d and dt, I see from the constructor, what those are. But when I have for instance a bug to fix and I know it's in updateBalls, I read the following line: balls.get(i).updatePosition(dt, g, d);. I have only a clue about what dt, g and d would be. If you'd name it better, it could read updatePosition(deltaTime, gravity, airResistance) which is much more clear.

Also be consistent. In Scene.setDrag the name of the parameter is airResistance, which will be assigned to d. So you have three different names for the same thing which makes it much harder to read.

double precision

I recommend to use BigDecimal instead of doubles, because of the preciscion problem, I think that could be a problem in the future. See wiki's "Floating Point" page, why I mention that.

direction / position

You might want to wrap direction and position into vectors. The main benefit would be, that you have a lot less parameters to pass and set, and it's usually quite clear, what a Vector is. Also not really sure if the calculation of the positions should be within the Projectile, since those are very common calculations - you might want to calculate those in a separate "MyMaths" type, or even in a Vector itself, maybe. The "maybe 3d in the future" problem should then be easier to implement (I have to admit, I suck at maths, so not quite sure if that would really help)

Other

  • Always use the interfaces, e.g. in Scene you declare balls as ArrayList<> instead of List<>. If you have to change to another implementation, you have a lot to change.
  • Scene.inFlux: Beside the fact I don't understand what "inFlux" means: Return !balls.isEmpty() should to the job.
  • In ProjectMotionApp you declare Timer timer;, it's then not used a few lines, and then you assign an instance to it. First of all, that's not necessary, second: Try to declare your variables where you actually need them. Because when I read timer = new Timer(), I will ask myself "Why is a new Timer created? What happend to the old one?".
  • In the GUI constructor, you create a new Scene with 9.8, 0, deltaTime. The remove the comment, you can declare 9.8 and 0 as constants, e.g. GRAVITIY and DEFAULT_AIR_RESISTANCE. Also: I think you shouldn't instantiate the Scene up there. I would first read all the properties, and then create a new Scene object.
  • The creation of the Projectile in the GUI is not clear. What is 10? What is scene.getHeight() - (scene.getHeight() / 16)? I'd suggest to declare it as a separate variable and give it a proper name, it can be ver helpful.
  • Scene.updateBalls(): Why are you removing the ball? Maybe you want to add a method isBallWithinScene or something, to make that clear.

Hope that helps ...

| improve this answer | |
\$\endgroup\$
  • 1
    \$\begingroup\$ Sorry but you rather lost all credibility with me when you said to use BigDecimal for a physics simulation. I think you should remove that recommendation. \$\endgroup\$ – markspace May 17 '17 at 18:21
  • \$\begingroup\$ @slowy Thanks. That's a lot of helpful information and more/less what I was looking for(I didn't know how to ask what I was looking for really. Two more questions. If not swing, what toolkit would you use? By vectors in direction / position do you I should implement my own or is there a java class for that? Thanks \$\endgroup\$ – defoification May 18 '17 at 1:32
  • \$\begingroup\$ @markspace why wouldn't I want the precision of Big Decimal? Should I be more concerned with storage space rather than the minor boost in accuracy? \$\endgroup\$ – defoification May 18 '17 at 1:36
  • \$\begingroup\$ @markspace: Please explain, why not to use BigDecimal for this use case, so I can copy paste that to my answer. \$\endgroup\$ – slowy May 18 '17 at 9:59
  • \$\begingroup\$ @defoification & slowy. You have to be kidding me. Do you not understand how numbers in a computer work? Floating point numbers like IEEE-754 were specifically designed with scientific problems in mind. BigDecimal has no concept of NAN or Infinity. Here's a question for you: what does BigDecimal's "accuracy" buy you? Show me the difference in an actual calculation where it make sense. \$\endgroup\$ – markspace May 18 '17 at 14: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.