java threads + bouncing balls

Question

Thanks in advance for help I created a program that makes multiple bouncing balls When user clicks on the screen a new ball should appear and move around screen. But when i click on the screen a ball appears and doesn't moving at all. When another click happens, the ball created previously jumped to another position instantly.

this is the ball class: used to create balls

import java.awt.Color;
import java.awt.Graphics2D;
import java.awt.geom.Ellipse2D;
import java.util.Random;
import javax.swing.JComponent;

public class Ball extends JComponent implements Runnable{
    private final int DIAMETER = 25;
    private final int v1 = 5;
    private final int v2 = -5;
    private final Random rnd = new Random();
    private int posX;
    private int posY;
    private Color color;
    private int xVelocity;
    private int yVelocity;
    
    public Ball(int posX, int posY) {
        this.posX = posX;
        this.posY = posY;
        this.color = randomColor(); 
        this.xVelocity = rnd.nextBoolean()?v1:v2;
        this.yVelocity = rnd.nextBoolean()?v1:v2;
    }
    
    public void move() {
        if (posX < 15) {
            xVelocity = -xVelocity;
        } else if(posX > 475) {
            xVelocity = -xVelocity;
        }
        
        if (posY < 0) {
            yVelocity = -yVelocity;
        } else if(posY > 475) {
            yVelocity = -yVelocity;
        }
        posX +=xVelocity;
        posY +=yVelocity;

    }
    
    public void draw(Graphics2D g2) {
        g2.setColor(color);
        g2.fill(new Ellipse2D.Double(posX,posY,DIAMETER,DIAMETER));
    }
    
    private static Color randomColor() {
        int r = (int)(Math.random()*255);
        int g = (int)(Math.random()*255);
        int b = (int)(Math.random()*255);
        Color rColor = new Color(r,g,b);
        return rColor;
    }

    @Override
    public void run() {
            while(!Thread.interrupted()) {
            move();
            repaint();
            try {
                Thread.sleep(60);
            } catch (InterruptedException ex) {}
            }

        
    }
}

this is the ballcomponent class: used to create the panel & display the balls

import java.awt.Color;
import java.awt.Dimension;
import java.awt.Graphics;
import java.awt.Graphics2D;
import java.awt.event.MouseAdapter;
import java.awt.event.MouseEvent;
import java.util.ArrayList;

import javax.swing.JFrame;
import javax.swing.JPanel;

public class BallComponent extends JPanel{
    private ArrayList<Ball> bList;
    
    public BallComponent() {
        bList = new ArrayList<Ball>();
        this.setPreferredSize(new Dimension(500,500));
        this.setBackground(Color.BLACK);
        this.addMouseListener(new ClickListener());
    }
    
    public void paintComponent(Graphics g) {
        Graphics2D g2 = (Graphics2D)g;
        super.paintComponent(g2);
        for (Ball a : bList) {
            a.draw(g2);
        }
    }
    
    private class ClickListener extends MouseAdapter{
        public void mouseClicked(MouseEvent e) {
            Ball a = new Ball(e.getX(),e.getY());
            bList.add(a);
            repaint();
            Thread gameThread = new Thread(a);
            gameThread.start();
        }
    }
    
    public static void main(String[] args) {
        JFrame frame = new JFrame();
        frame.setTitle("Bouncing Balls");
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        frame.add(new BallComponent());
        frame.pack();
        frame.setVisible(true);
    }
}

I am seeing this same mistake too many times recently where you make a graphics class extend a GUI component unnecessarily, and it is hurting you. The answer is ***don't***. Don't do this. The Ball class should definitely *not* extend JComponent (nor should it implement Runnable). It should be a logical class (non-component class) and only one GUI component, probably a JPanel should be involved in the drawing. Also there should a single game-thread, not multiple, and best implemented with a Swing Timer. — Hovercraft Full Of Eels, Mar 23 '21 at 16:14
Don't keep creating multiple Threads. Instead your application should use a single Swing Timer. When the Timer fires you invoke move() and repaint() on your BallComponent class and all the Balls in the List should be painted. The Balls should not be actual components. Instead just use Graphics to paint each Ball. See https://stackoverflow.com/questions/54028090/get-width-and-height-of-jpanel-outside-of-the-class/54028681#54028681 for an example of this approach. — camickr, Mar 23 '21 at 16:15
Thx guys, but creating multiple threads & not using a timer is what I'm asked to do, so I've got no choice.... — Yu Hu, Mar 23 '21 at 16:21
You are calling repaint on the Ball. That doesn't call repaint on the panel with the balls. The fact your ball is a JComponent is completely unused. You need to call BallComponent.repaint. Which you do when you add a new ball. — matt, Mar 23 '21 at 16:30

score 2 · Accepted Answer · answered Mar 23 '21 at 16:38

2

Since you have a bunch of threads doing random stuff. You might as well have your JPanel update on it's own.

At the end of your main method.

new Thread( ()->{
    while(true){
        frame.repaint();
        try{
            Thread.sleep(60);
        } catch(Exception e){
            break;
        }
    }
}).start();

Usually you would do this with a timer task, but since you said you couldn't use a Timer, I just wrote a thread version.

I think repaint() is safe to call from off of the EDT since it just schedules a repaint. The paintComponent method and click method will only be called on the EDT. So you shouldn't get CCME. There are a bunch of race conditions with the multiple threads, but it seems like the only problem would be balls drawn out of position.

answered Mar 23 '21 at 16:38

matt

10,892
3
22
34

thx mate, that's absolutely a wonderful solution is it possible to do this some other way? is it possible to add some code in my mouseClicked() method instead of at the end of the main method? – Yu Hu Mar 23 '21 at 16:41
@YuHu why do it in your mouse clicked method? You *can* add it there if you want. – matt Mar 23 '21 at 19:38
1

Just a note: you don't have to start a new `Thread` in main, because you already have the main `Thread` itself. So you can just implement the loop inside the main method (after showing the GUI). Also note that instead of looping for ever and sleeping manually, you can use a `ScheduledExecutorService#scheduleAtFixedRate`, because sleeping in a loop is not a good practice as far as I have read. – gthanop Mar 24 '21 at 13:56

java threads + bouncing balls

1 Answers1