Java Bouncing Ball Review

Question

Just looking for some help reviewing/refactoring. For example, could the classes random/vertical be refactored into 1 class instead of 2?

BouncingBalls.java

import java.awt.BorderLayout;
import java.awt.Dimension;
import java.awt.Graphics;
import java.awt.Toolkit;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.awt.event.MouseEvent;
import java.awt.event.MouseListener;
import java.util.ArrayList;
import java.util.List;
import javax.swing.JFrame;
import javax.swing.JPanel;
import javax.swing.Timer;
public class BouncingBalls extends JPanel implements MouseListener {
 protected List<RandomBall> randomBalls = new ArrayList<RandomBall>(20);
 protected List<VerticalBall> verticalBalls = new ArrayList<VerticalBall>(20);
 private Container container;
 private DrawCanvas canvas;
 private Boolean doubleClick = false;
 private final Integer waitTime = (Integer) Toolkit.getDefaultToolkit()
 .getDesktopProperty("awt.multiClickInterval");
 private static int canvasWidth = 500;
 private static int canvasHeight = 500;
 public static final int UPDATE_RATE = 30;
 int count = 0;
 public static int random(int maxRange) {
 return (int) Math.round((Math.random() * maxRange));
 }
 public BouncingBalls(int width, int height) {
 canvasWidth = width;
 canvasHeight = height;
 container = new Container();
 canvas = new DrawCanvas();
 this.setLayout(new BorderLayout());
 this.add(canvas, BorderLayout.CENTER);
 this.addMouseListener(this);
 start();
 }
 public void start() {
 Thread t = new Thread() {
 public void run() {
 while (true) {
 update();
 repaint();
 try {
 Thread.sleep(1000 / UPDATE_RATE);
 } catch (InterruptedException e) {
 }
 }
 }
 };
 t.start();
 }
 public void update() {
 for (RandomBall ball : randomBalls) {
 ball.ballBounce(container);
 }
 for (VerticalBall ball : verticalBalls) {
 ball.verticalBounce(container);
 }
 }
 class DrawCanvas extends JPanel {
 public void paintComponent(Graphics g) {
 super.paintComponent(g);
 container.draw(g);
 for (RandomBall ball : randomBalls) {
 ball.draw(g);
 }
 for (VerticalBall ball : verticalBalls) {
 ball.draw(g);
 }
 }
 public Dimension getPreferredSize() {
 return (new Dimension(canvasWidth, canvasHeight));
 }
 }
 public static void main(String[] args) {
 javax.swing.SwingUtilities.invokeLater(new Runnable() {
 public void run() {
 JFrame f = new JFrame("Stack Answer 2");
 f.setDefaultCloseOperation(f.EXIT_ON_CLOSE);
 f.setContentPane(new BouncingBalls(canvasHeight, canvasWidth));
 f.pack();
 f.setVisible(true);
 }
 });
 }
 @Override
 public void mouseClicked(MouseEvent e) {
 // TODO Auto-generated method stub
 }
 @Override
 public void mouseEntered(MouseEvent e) {
 // TODO Auto-generated method stub
 }
 @Override
 public void mouseExited(MouseEvent e) {
 // TODO Auto-generated method stub
 }
 @Override
public void mousePressed(MouseEvent e) {
 final int x = e.getX();
 final int y = e.getY();
 if (e.getClickCount() >= 2) {
 doubleClick = true;
 verticalBalls.add(new VerticalBall(x, y, canvasWidth, canvasHeight));
 System.out.println("double click");
 } else {
 Timer timer = new Timer(waitTime, new ActionListener() {
 public void actionPerformed(ActionEvent e) {
 if (doubleClick) {
 /* we are the first click of a double click */
 doubleClick = false;
 } else {
 count++;
 randomBalls.add(new RandomBall(x, y, canvasWidth, canvasHeight));
 /* the second click never happened */
 System.out.println("single click");
 }
 }
 });
 timer.setRepeats(false);
 timer.start();
 }
}
 @Override
 public void mouseReleased(MouseEvent e) {
 // TODO Auto-generated method stub
 }
}

RandomBall.java

import java.awt.Color;
import java.awt.Graphics;
public class RandomBall {
 public static int random(int maxRange) {
 return (int) Math.round((Math.random() * maxRange));
 }
 private int x;
 private int y;
 private int canvasWidth = 500;
 private int canvasHeight = 500;
 private boolean leftRight;
 private boolean upDown;
 private int deltaX;
 private int deltaY;
 private int radius = 20;
 private int red = random(255);
 private int green = random(255);
 private int blue = random(255);
 RandomBall(int x, int y, int width, int height) {
 this(x, y, width, height, false, false);
 }
 RandomBall(int x, int y, int width, int height, boolean leftRight, boolean upDown) {
 this.x = x;
 this.y = y;
 this.canvasWidth = width;
 this.canvasHeight = height;
 this.leftRight = leftRight;
 this.upDown = upDown;
 updateDelta();
 }
 public void draw(Graphics g) {
 g.setColor(new Color(red, green, blue));
 g.fillOval((int) (x - radius), (int) (y - radius), (int) (2 * radius),
 (int) (2 * radius));
 }
 private void updateDelta() {
 final int minimumMovement = 5;
 final int maxExtra = 10;
 deltaY = minimumMovement + (int) (Math.random() * maxExtra);
 deltaX = minimumMovement + (int) (Math.random() * maxExtra);
 }
 public void ballBounce(Container container) {
 // controls horizontal ball motion
 if (leftRight) {
 x += deltaX;
 if (x >= getWidth()) {
 leftRight = false;
 updateDelta();
 }
 } else {
 x += -deltaX;
 if (x <= 0) {
 leftRight = true;
 updateDelta();
 }
 }
 // controls vertical ball motion
 if (upDown) {
 y += deltaY;
 if (y >= getHeight()) {
 upDown = false;
 updateDelta();
 }
 } else {
 y += -deltaY;
 if (y <= 0) {
 upDown = true;
 updateDelta();
 }
 }
 }
 public void verticalBounce(Container container) {
 // controls vertical ball motion
 if (upDown) {
 y += deltaY;
 if (y >= getHeight()) {
 upDown = false;
 updateDelta();
 }
 } else {
 y += -deltaY;
 if (y <= 0) {
 upDown = true;
 updateDelta();
 }
 }
 }
 public int getX() {
 return x;
 }
 public int getY() {
 return y;
 }
 public int getWidth() {
 return canvasWidth;
 }
 public int getHeight() {
 return canvasHeight;
 }
}

VerticalBall.java

import java.awt.Color;
import java.awt.Graphics;
public class VerticalBall {
 public static int random(int maxRange) {
 return (int) Math.round((Math.random() * maxRange));
 }
 private int x;
 private int y;
 private int canvasWidth = 500;
 private int canvasHeight = 500;
 private boolean upDown;
 private int deltaY;
 private int radius = 20;
 private int red = random(255);
 private int green = random(255);
 private int blue = random(255);
 VerticalBall(int x, int y, int width, int height) {
 this(x, y, width, height, false);
 }
 VerticalBall(int x, int y, int width, int height, boolean upDown) {
 this.x = x;
 this.y = y;
 this.canvasWidth = width;
 this.canvasHeight = height;
 this.upDown = upDown;
 updateDelta();
 }
 public void draw(Graphics g) {
 g.setColor(new Color(red, green, blue));
 g.fillOval((int) (x - radius), (int) (y - radius), (int) (2 * radius),
 (int) (2 * radius));
 }
 private void updateDelta() {
 final int minimumMovement = 5;
 final int maxExtra = 10;
 deltaY = minimumMovement + (int) (Math.random() * maxExtra);
 }
 public void verticalBounce(Container container) {
 // controls vertical ball motion
 if (upDown) {
 y += deltaY;
 if (y >= getHeight()) {
 upDown = false;
 updateDelta();
 }
 } else {
 y += -deltaY;
 if (y <= 0) {
 upDown = true;
 updateDelta();
 }
 }
 }
 public int getX() {
 return x;
 }
 public int getY() {
 return y;
 }
 public int getWidth() {
 return canvasWidth;
 }
 public int getHeight() {
 return canvasHeight;
 }
}

Container.java

import java.awt.Color;
import java.awt.Graphics;
 public class Container {
 private static final int HEIGHT = 500;
 private static final int WIDTH = 500;
 private static final Color COLOR = Color.WHITE;
 public void draw(Graphics g) {
 g.setColor(COLOR);
 g.fillRect(0, 0, WIDTH, HEIGHT);
 }
 }

Answer 1

Your VerticalBall class and RandomBall have lots of duplicate code(should it?). So you can make an abstract Ball class and define the duplicate codes there. This way you can have a clear hierarchy of classes.

See : re-factoring also.

BTW in Java final variable names are written like CAPITAL_WITH_UNDERSCORES.

Answer 2

i believe that the main issue here is code duplication, like tintinmj already said.

also, here s a good article about different kinds of game loops. yours seems to be not scaling according to real time passed since the last game update.

 g.setColor(new Color(red, green, blue));

do you really need to create new instance of Color every time here?

in RandomBall class, there's code duplication in ballBounce and verticalBounce. probably first one should call the second instead.

public getters in *Ball classes seems to be useless for now?