GUI challenge: two balls

[Alex Chun]

Hi everyone. My challenge.
Write a Java Program that creates a window with a check box named “Collision” at
its bottom and a red ball moving horizontally at its center.
If the check box is unchecked, the ball keeps moving in one direction until it collides
with the walls on the other side, after which it starts moving in the opposite
direction. The ball should be updated using a Runnable (3 Points).
However, when the checkbox is checked, a blue ball appears at any random point
along the same line as the red circle.
The blue ball does not move. The red ball keeps moving in one direction until it collides
with the blue ball. After collision it starts moving in the opposite direction. Unchecking
the box makes the blue ball disappear and the application returns to its normal mode. (3 Points )

See video below.

[Alex Chun]

I have two classes. Here is the red ball is moving only. I do not know how to realize a blue ball.

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

/**
 * Created by Crazy on 05.08.2015.
 */
public class SimpleAnimation implements ActionListener {
    private DrawPanel dp = new DrawPanel();
    static JCheckBox cb = new JCheckBox();

    private JFrame frame;
    public void go() {
        frame = new JFrame();
        frame.setTitle("Simple Animation");
        frame.setSize(600, 600);
        cb.setText("Collision");
        frame.getContentPane().add(BorderLayout.CENTER, dp);
        frame.getContentPane().add(BorderLayout.SOUTH, cb);
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        frame.setVisible(true);
        move();
        frame.pack();
        cb.addActionListener(this);
    }

    public void actionPerformed(ActionEvent ev) {
        frame.repaint();
    }

    public void move() {
            int x = 0;
            int dx = 1;
            //System.out.println(frame.getWidth());
            while(true) {
                  dp.setX(x += dx);
                if (x == 250 /*250 frame.getWidth()*/)
                    dx = -1;
                if (x == 0)
                    dx = 1;
                frame.repaint();
                try {
                    Thread.sleep(5);
                } catch (Exception e) {}
            }
    }

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

    }
}

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

/**
 * Created by Crazy on 05.08.2015.
 */
public class DrawPanel extends JPanel {

    private int x;
    static Random random = new Random();
    int randomX;
    public int getX() {
        return x;
    }

    public void setX(int x) {
        this.x = x;
    }

    public void paintComponent(Graphics g) {
        g.setColor(Color.RED);
        g.fillOval(x, 200, 80, 80);
}

[Campbell Ritchie]

Alex Chun wrote: . . . The ball should be updated using a Runnable (3 Points).
. . .

I trust you know why that is incorrect? You should not use more than one thread for a Swing app like that. You should use a Timer.

You have gone one worse, I am afraid. You have neither taken the standard route of running everything on the EDT and using a Timer, nor the incorrect route in the question. You have called sleep() on the current thread. All you achieve like that is freezing the display. Since the sleep() call is short, the 5ms delay may be less than the time it takes to repaint your display. At the moment it seems you are getting away with it.

I think you should query the question because it violates thread safety. Then you will probably have to write a Runnable to move the red ball. If you don't, you will lose marks.

I also think you should have a Ball class. Give it another Ball as a field and you can test for collisions between the two.

[Alex Chun]

I'm afraid that I don't know why not. Just I started to learn Swing/AWT. I' m agree about class "Ball" creation.

PS. I updated the link and free access.
https://youtu.be/hvNAUUP_mS0

[Campbell Ritchie]

Yes, the link works now. Thank you.

Start with the ball class and give it size colour and location fields. Location will probably count as two fields x and y.
Give it move methods and a field for the other ball. Give it methods to make the other ball appear and disappear. Give it a collide method.
Hint: collision means separation between the centres of the two balls is ≤ sum of their radii.
The direction of movement changes whenever there is a collision, so you would need a distance field which changes sign when you collide. In the case of the blue ball, make the distance moved 0.

If two balls are created so they are overlapping, your collide method will not work. In that case let the new ball throw an Exception from its constructor.

[Campbell Ritchie]

Swing is not thread‑safe. You ought never to manipulate Swing components in other threads. Creating a Runnable means another thread, and that is very error‑prone.

[Alex Chun]

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

/**
 * Created by Crazy on 05.08.2015.
 */
public class Ball extends JPanel{

    static Random random = new Random();
    private Color colour;
    private int radius;
    private int x = random.nextInt(250);
    private int y = 200;
    private Ball otherBall;
    private int direction = 1;

    public Ball() {
        try {
            if (this.x + radius < otherBall.x && (this.x - radius) > otherBall.x)
                throw new WrongBallLocationException();
        } catch(WrongBallLocationException e) {}
    }

    public void paintComponent(Graphics g) {
        g.setColor(colour);
        g.fillOval(x, y, radius, radius);
    }

    public void otherBallAppear() {
        //what is here?
        if (SimpleAnimation.cb.isSelected())
            this.otherBall = new Ball();
    }

    public void otherBallDisappear() {
        //what is here?
        if (!SimpleAnimation.cb.isSelected())
            this.otherBall = null;
    }

    public void collide() {
        //what is here?
        if (this.getX() + radius == this.otherBall.getX() || otherBall.getX() + otherBall.radius == this.getX() || x == 250 || x == 0)
            this.direction = -direction;
    }

    public void move(JFrame frame) {
        while(true) {
            x += direction;
            collide();
            frame.repaint();
            try {
                Thread.sleep(5);
            } catch (Exception e) {}
        }
    }

    public Ball getBall() {
        return otherBall;
    }

    public void setBall(Ball ball) {
        this.otherBall = ball;
    }

    public int getX() {
        return x;
    }

    public void setX(int x) {
        this.x = x;
    }

    public int getRadius() {
        return radius;
    }

    public void setRadius(int radius) {
        this.radius = radius;
    }

    public Color getColour() {
        return colour;
    }

    public void setColour(Color colour) {
        this.colour = colour;
    }

    public int getY() {
        return y;
    }

    public void setY(int y) {
        this.y = y;
    }
}

[Campbell Ritchie]

Alex Chun wrote:

 . . .
public class Ball extends JPanel
. . .

Nonononononono.

It is

class Ball
{

No extends, or if you insist, extends Object. It might need public access.
Ball should be a class in its own right, not a subclass of a GUI display class.

[Alex Chun]

How can I draw two balls?

[Campbell Ritchie]

One with one ball object and one with the other ball object.

[Piet Souris]

Campbell Ritchie wrote:(...)I trust you know why that is incorrect? You should not use more than one thread for a Swing app like that. You should use a Timer.

You have gone one worse, I am afraid. You have neither taken the standard route of running everything on the EDT and using a Timer, nor the incorrect route in the question. You have called sleep() on the current thread. All you achieve like that is freezing the display. Since the sleep() call is short, the 5ms delay may be less than the time it takes to repaint your display. At the moment it seems you are getting away with it.

Yesterday I tried to explain why this statement is incorrect. Either my remarks were unclear or
you don't believe what I wrote, so here is a little demo. It lets the thread in question
sleep for 5 seconds, then changes the panels redraw color.
While it is asleep, move the panel, and especially resize it; that will force the system
to skip the double buffering and so it will invoke the paintComponent. Do you notice
any slowdown at all? I also print out whether we are running on the EDT or not.

And indeed: there is a risk of memory inconsistency, but as I said: the chance
of this happening is, in this case, minimal. So indeed, this program should be
running from the EDT, and a Timer is the thing to use. But a Runnable is also
oke. Just a tad more risky.

public class DemoSleep {
    public static void main(String... args) {
        new DemoSleep.move();
    }
    
    int width = 500, height = 500;
    double ballRatio = .1;
    int x = 100, y = 250;
    Color color = Color.blue;
    JPanel panel;
    
    public DemoSleep() {
        panel = new JPanel() {
            @Override
            public void paintComponent(Graphics g) {
                System.out.println("About to repaint the panel. Is this the EDT? " + SwingUtilities.isEventDispatchThread());
                super.paintComponent(g);
                int w = this.getWidth();
                int h = this.getHeight();
                int x = (int) ((.5 - ballRatio / 2) * w + .5);
                int y = (int) ((.5 - ballRatio / 2) * h + .5);
                g.setColor(color);
                g.fillOval(x, y, (int) (ballRatio * w + .5), (int) (ballRatio * h + .5));
                g.drawString("is this EDT? " + SwingUtilities.isEventDispatchThread(), 10, 50);
            }
        };
        panel.setPreferredSize(new Dimension(width, height));
        JFrame frame = new JFrame("Sleep demo");
        frame.getContentPane().add(panel, BorderLayout.CENTER);
        frame.pack();
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        frame.setLocationRelativeTo(null);
        frame.setVisible(true);
    }
    
    private void move() {
        // no moving, just delaying the panel !?!?!?
        Random r = new Random();
        while (true) {
            try {
           Thread.sleep(5000);
        }
        catch (Exception s) {
            System.out.println("Interrupted!");
            return;
        }
        System.out.println("Just woke up. Is this the EDT? " + SwingUtilities.isEventDispatchThread());
        color = new Color(r.nextInt(256), r.nextInt(256), r.nextInt(256));
        panel.repaint();
        }
    }
}

Greetz,
Piet

[Campbell Ritchie]

Piet Souris wrote: . . . Either my remarks were unclear or
you don't believe what I wrote, . . .

Or believe they ought to be taught to do things right.

It may only be a slight risk, and yes, I did run the program and saw that OP had got away with it. We are not interested in getting away with things. We want to see people do things right. If you get away with it where it is trivial (here) you will think you can get away with it where it matters.
Take an analogy with driving. If you repeatedly run the lights you will think you can get away with it. Until one day you cause an accident and kill somebody.

The difference between bad driving and bad programming is that you can't harm 1000000 people all at once by bad driving.

The approach OP has taken is worse than using a Runnable and worse than using a Timer. Using a Timer would have been the correct approach, but OP has not done that. The question says to use a Runnable, and he has not done that either. So he will produce poor quality code because he has simply used Thread#sloop, and also lose all those 3 marks because he hasn't done what the question asks for. And yes, there is a 99.99% chance that he will get away with the poor coding; I tried the code yesterday and the red ball went left and right. And nothing else went wrong.

Actually something else did go wrong, which I shall point out when we see the next version of the Ball class.

[Piet Souris]

My reply only is about the Tread.sleep and whether it does or does not slow down
the EDT.

My impression is also that this is an assignment OP has to fullfill, and that he must
use a Runnable (that must use Thread.sleep ;))))

But I leave that whole discussion up to you. Starting from the EDT, using a dedicated
BALL class, it makes perfect sense. My only advice would be to get it absolutely
clear if OP is allowed to use a timer or not. If not: I'm curious where this topic will
lead us to!

Greetz,
Piet

[Campbell Ritchie]

Piet Souris wrote: . . .
My impression is also that this is an assignment OP has to fullfill, and that he must
use a Runnable (that must use Thread.sleep ;))))
. . .

Yes, that would appear to be the case.

I think the best outcome is to persuade them to change the question, but I think that is impossible after the assignment has been given.