1
\$\begingroup\$

I wrote a simple animation of a ball moving across the screen. I was wondering if I used the static variable correctly. If not, how could I make it non-static so I could instantiate it in the main method whilst being able to access its getter methods in the listener inner class? Also, did I use the SwingUtilities.invokeLater() method properly?

Please let me know of any improvements that I can make.

import java.awt.*;
import javax.swing.*;

public class SimpleAnimation {
    private JFrame frame;
    private int x = 5;
    private int y = 5;

    static SimpleAnimation animation;

    public static void main(String[] args){
        animation = new SimpleAnimation();
        animation.buildGUI();

        // Use i to change co-ordinates x and y to produce a moving ball on the screen.
        for(int i = 0; i <= 300; i+=1){
            animation.setCoordinates(i, i);
            animation.frame.repaint();      // JVM uses new co-ordinates in paintComponent() to repaint the ball
            try{
                Thread.sleep(10);
            }catch(InterruptedException interruptedException){
                interruptedException.printStackTrace();
            }
        }
    }

    public void buildGUI(){
        frame = new JFrame("Simple animation");
        SwingUtilities.invokeLater(
                new Runnable() {
                    @Override
                    public void run() {
                        frame.setContentPane(new DrawPanel(new BorderLayout()));
                        frame.setSize(400, 500);
                        frame.setDefaultCloseOperation(WindowConstants.EXIT_ON_CLOSE);
                        frame.setLocationRelativeTo(null);
                        frame.setVisible(true);
                    }
                }
        );

    }

    // GETTERS
    int getX(){
        return x;
    }

    int getY(){
        return y;
    }

    // SETTERS
    void setCoordinates(int newX, int newY){
        x = newX;
        y = newY;
    }

    class DrawPanel extends JPanel{
        public DrawPanel(LayoutManager layout){
            super(layout);
        }

        @Override
        public void paintComponent(Graphics g){
            g.setColor(Color.ORANGE);
            g.fillOval(animation.getX(), animation.getY(), 100, 100);
        }
    }
}
\$\endgroup\$
1
\$\begingroup\$

I was wondering if I used the static variable correctly? If not, how could I make it non-static so I could instantiate it in the main method whilst being able to access its getter methods in the listener inner class?

I would prefer to not use a static variable for this, which you could easily achieve by passing the animation object to the panel:

public class AnimationPanel extends JPanel {

    private SimpleAnimation animation;

    public AnimationPanel(LayoutManager layout, SimpleAnimation animation) {
        super(layout);
        this.animation = animation;
    }

    @Override
    public void paintComponent(Graphics g) {
        super.paintComponent(g);
        g.setColor(Color.ORANGE);
        g.fillOval(animation.getX(), animation.getY(), 100, 100);
    }
}

With a simple animation class (in a real project, setcoordinates would more likely be updateCoordinates, as the animation model should contain its logic itself, ie it should know what to animate, instead of having the main class/controller manage this. the main class would just be a loop constantly calling updateCoordinates on the model, and repaint on the view):

public class SimpleAnimation {
    private int x = 5;
    private int y = 5;

    int getX(){
        return x;
    }

    int getY(){
        return y;
    }

    void setCoordinates(int newX, int newY){
        x = newX;
        y = newY;
    }
}

And then use it in your main like this:

public class AnimationMain {

    public static void main(String[] args) {
        SimpleAnimation animation = new SimpleAnimation();
        AnimationPanel animationPanel = new AnimationPanel(new BorderLayout(), animation);

        buildGUI(animation, animationPanel);

        for (int i = 0; i <= 300; i += 1) {
            animation.setCoordinates(i, i);
            animationPanel.repaint();
            try {
                Thread.sleep(10);
            } catch (InterruptedException interruptedException) {
                interruptedException.printStackTrace();
            }
        }
    }

    public static void buildGUI(SimpleAnimation animation, AnimationPanel animationPanel) {
        JFrame frame = new JFrame("Simple animation");
        SwingUtilities.invokeLater(
                new Runnable() {
                    @Override
                    public void run() {
                        frame.setContentPane(animationPanel);
                        frame.setSize(400, 500);
                        frame.setDefaultCloseOperation(WindowConstants.EXIT_ON_CLOSE);
                        frame.setLocationRelativeTo(null);
                        frame.setVisible(true);
                    }
                }
        );
    }
}

The main reason I would prefer this approach is that it follows the MVC approach (separating the code by model (SimpleAnimation), view (AnimationPanel), and controller (AnimationMain)). This seems a lot more flexible (with your approach, I even have problems extracting the main method to its own class, for example).

Misc

  • don't import *, instead make all your imports explicit, so readers know what classes you use.
  • your methods should be explicitly declared as private or public.
  • your spacing is sometimes off (eg i+=1; it should be i += 1, or the more customary i++).
\$\endgroup\$
  • \$\begingroup\$ Great, thanks for this! It's been really helpful. I'd just like to ask if it would be a better idea to have buildGUI() as a non-static method? Also, after reading up on the MVC approach, I was wondering if it would be a good idea to put the buildGUI() into its own class or for it to stay in AnimationMain? Thanks for your help :) \$\endgroup\$ – Calculus5000 Mar 5 '15 at 16:01
  • \$\begingroup\$ I forgot to ask, in the AnimationPanel, super is invoked in the overridden paintComponent() method. I was wondering what was the rationale behind this? Cheers \$\endgroup\$ – Calculus5000 Mar 5 '15 at 16:08
  • 1
    \$\begingroup\$ @Calculus5000 yes, both of those things would probably be better. I didn't do it because its basically just the entry point/example case, so it doesn't really matter. And the super call repaints the whole thing. Without it, previous versions of your objects stay painted (so you basically have a yellow path instead of a ball). I'm not quite sure why you didn't need it in your original version (might be a nice question for stackoverflow). \$\endgroup\$ – tim Mar 5 '15 at 16:15

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.