Why does the thread freeze my code in my actionListener implementation?

Question

I am trying to make an oval move smoothly by taping the buttons (up,down,...)and I try to use threads with the try and catch it's work inside the class but when I use it here it freezes the whole app.

class movingdown implements ActionListener{
    @Override 
    public void actionPerformed(ActionEvent e){
        for (int i = 0; i < frame.getWidth(); i++) {
            x++;
            frame.repaint();
            try{Thread.sleep(50);}catch(Exception ex){}

        }
    }
}//movingdown end

Here is my whole code:

package swingWork;
import java.awt.BorderLayout;
import java.awt.Button;
import java.awt.Color;
import java.awt.Graphics;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;

import javax.swing.*;
public class movingball {
JFrame frame;
int x=0;
int y =0;
public static void main(String []args){
    movingball ball = new movingball();
    ball.start();

}
public void start(){
    ballDrawer drawer = new ballDrawer();   
    frame = new JFrame();
    frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);

    JButton up =new JButton("UP");
    JButton down =new JButton("DOWN");
    JButton left =new JButton("LEFT");
    JButton right =new JButton("RIGHT");

    frame.getContentPane().add(BorderLayout.NORTH, up);
    frame.getContentPane().add(BorderLayout.SOUTH, down);   
    frame.getContentPane().add(BorderLayout.WEST, left);
    frame.getContentPane().add(BorderLayout.EAST, right);
    frame.getContentPane().add(BorderLayout.CENTER,drawer);
    //setting up the action listener:

    up.addActionListener(new movingup());
    down.addActionListener(new movingdown());
    left.addActionListener(new movingleft());
    right.addActionListener(new movingright());
    frame.setSize(300, 300);
    frame.setVisible(true);
int rightsize =right.getWidth();



}//start method end


//here we have the listeners

class movingup implements ActionListener{

    @Override
    public void actionPerformed(ActionEvent e) {
    for (int i = x; i > frame.getHeight(); i--) {
        x=i;
        try{Thread.sleep(50);}catch(Exception ex){}
    }
    }

}//moving up end
class movingdown implements ActionListener{
    @Override 
    public void actionPerformed(ActionEvent e){
        for (int i = 0; i < frame.getWidth(); i++) {
            x++;
            frame.repaint();
            try{Thread.sleep(50);}catch(Exception ex){}

        }
    }
}//movingdown end
class movingleft implements ActionListener{
    public void actionPerformed(ActionEvent e){


            x=x+10;
            frame.repaint();

        System.out.println(x);

    }
}//moving  left end
class movingright implements ActionListener{
    public void actionPerformed(ActionEvent e){

    }
}//moving right end


class ballDrawer extends JPanel{
    @Override
public void paintComponent(Graphics g) {
        g.setColor(Color.white);
        g.fillRect(0, 0, this.getWidth(), this.getHeight());

        g.setColor(Color.black);
        g.fillOval(x, y, 100, 100);


    }//PaintComponents End


}//ball drawer class end

}//main class end

Answer 1

As, has already been pointed out, you're blocking the Event Dispatching Thread, which is preventing the UI from been updated until your loop and method exit

A better solution is to use a Swing Timer, which generates regular callbacks to a ActionListener within the context of the EDT, allowing you to safely update the UI from within it. See Concurrency in Swing and How to use Swing Timers for more details

You should also be calling super.paintComponent, which, amongst other things, will fill the background of the Graphcis context for your automatically.

import java.awt.BorderLayout;
import java.awt.Color;
import java.awt.Dimension;
import java.awt.EventQueue;
import java.awt.Graphics;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import javax.swing.JButton;
import javax.swing.JFrame;
import javax.swing.JPanel;
import javax.swing.Timer;
import javax.swing.UIManager;
import javax.swing.UnsupportedLookAndFeelException;

public class Movingball {

    public static void main(String[] args) {
        EventQueue.invokeLater(new Runnable() {
            @Override
            public void run() {
                try {
                    UIManager.setLookAndFeel(UIManager.getSystemLookAndFeelClassName());
                } catch (ClassNotFoundException | InstantiationException | IllegalAccessException | UnsupportedLookAndFeelException ex) {
                    ex.printStackTrace();
                }

                Movingball ball = new Movingball();
                ball.start();
            }
        });
    }

    private BallDrawer drawer;

    public void start() {
        drawer = new BallDrawer();
        JFrame frame = new JFrame();
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);

        JButton up = new JButton("UP");
        JButton down = new JButton("DOWN");
        JButton left = new JButton("LEFT");
        JButton right = new JButton("RIGHT");

        frame.getContentPane().add(BorderLayout.NORTH, up);
        frame.getContentPane().add(BorderLayout.SOUTH, down);
        frame.getContentPane().add(BorderLayout.WEST, left);
        frame.getContentPane().add(BorderLayout.EAST, right);
        frame.getContentPane().add(BorderLayout.CENTER, drawer);
        // setting up the action listener:

        up.addActionListener(new Movingup());
        down.addActionListener(new Movingdown());
        left.addActionListener(new Movingleft());
        right.addActionListener(new Movingright());
        frame.pack();
        frame.setVisible(true);
    }// start method end

    // here we have the listeners
    class Movingup implements ActionListener {

        @Override
        public void actionPerformed(ActionEvent e) {
            drawer.moveUp();
        }
    }// moving up end

    class Movingdown implements ActionListener {

        @Override
        public void actionPerformed(ActionEvent e) {
            drawer.moveDown();
        }
    }// movingdown end

    class Movingleft implements ActionListener {

        @Override
        public void actionPerformed(ActionEvent e) {
            drawer.moveLeft();
        }
    }// moving left end

    class Movingright implements ActionListener {

        @Override
        public void actionPerformed(ActionEvent e) {
            drawer.moveRight();
        }
    }// moving right end

    public static class BallDrawer extends JPanel {

        public static final int CIRCLE_WIDTH = 100;
        public static final int CIRCLE_HEIGHT = 100;

        private int xDelta;
        private int yDelta;

        private int xPos;
        private int yPos;

        public BallDrawer() {
            Timer timer = new Timer(40, new ActionListener() {
                @Override
                public void actionPerformed(ActionEvent e) {
                    xPos += xDelta;
                    yPos += yDelta;

                    if (xPos < 0) {
                        xPos = 0;
                        xDelta = 0;
                    } else if (xPos + CIRCLE_WIDTH > getWidth()) {
                        xPos = getWidth() - CIRCLE_WIDTH;
                        xDelta = 0;
                    }
                    if (yPos < 0) {
                        yPos = 0;
                        yDelta = 0;
                    } else if (yPos + CIRCLE_HEIGHT > getHeight()) {
                        yPos = getHeight() - CIRCLE_HEIGHT;
                        yDelta = 0;
                    }

                    repaint();
                }
            });
            timer.start();
        }

        @Override
        public Dimension getPreferredSize() {
            return new Dimension(400, 400);
        }

        public void moveLeft() {
            xDelta = -1;
        }

        public void moveRight() {
            xDelta = 1;
        }

        public void moveUp() {
            yDelta = -1;
        }

        public void moveDown() {
            yDelta = 1;
        }

        @Override
        protected void paintComponent(Graphics g) {
            super.paintComponent(g);

            g.setColor(Color.black);
            g.fillOval(xPos, yPos, CIRCLE_WIDTH, CIRCLE_HEIGHT);

        }// PaintComponents End

    }// ball drawer class end

}// main class end

You should also isolate functionality to the class responsible for actually managing it, in this your BallDrawer should be managing the movement of the ball, not the other class, this allows it to be self contained and re-usable.

Answer 2

You never, ever, EVER want to use Thread.sleep() on the Event Dispatch Thread. EVER!!

It never accomplishes anything good, and it makes your screen freeze until the sleep is over. So don't do it!!

The best way to animate in Swing is to use a javax.swing.Timer to fire events that change your drawing properties periodically. This happens on a different thread so the screen stays responsive. You can set this timer to run all the time, but have it do nothing when the circle should not be moving. Here's the code for the Timer's ActionListener

t = new Timer(TIMER_DELAY, new ActionListener() {
  @Override
  public void actionPerformed(ActionEvent e) {
    if(deltaX == 1 && x >= frame.getWidth() - CIRCLE_WIDTH) deltaX = 0;
    if(deltaY == 1 && y >= frame.getHeight() - CIRCLE_HEIGHT) deltaY = 0;
    if(deltaX == -1 && x <= 0) deltaX = 0;
    if(deltaY == -1 && y <= 0) deltaY = 0;
    x += deltaX;
    y += deltaY;
    if(deltaX != 0 || deltaY != 0) {
      frame.repaint();
    }
  }
});
t.start();

Then, your buttons just tell the timer to animate in the correct direction by setting the delta value, e.g.

class movingup implements ActionListener {
  @Override
  public void actionPerformed(ActionEvent e) {
    deltaY = -1;
  }
}// moving up end

As you can see, I create a timer object inside start method, and just tell the timer to start animating by pressing the button. The timer has it's own, different ActionListener (implemented as an anonymous class here) that actually tells the parameters to change and then repaints if something has moved. You could optimize it further by telling Swing to only redraw the part of the screen that needs redrawing with JComponent.repaint(Rectangle), but this is just an example. If you do this, make sure the repaint is large enough to get both where the circle was and where it is now.

Here's the complete program, including the constants CIRCLE_WIDTH and CIRCLE_HEIGHT, the new timer logic, and all four buttons changed:

package swingWork;

import java.awt.BorderLayout;
import java.awt.Button;
import java.awt.Color;
import java.awt.Graphics;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;

import javax.swing.*;

public class movingball {
  private static final int CIRCLE_WIDTH = 100;
  private static final int CIRCLE_HEIGHT = 100;
  private static final int TIMER_DELAY = 50;

  JFrame frame;
  int x = 0;
  int y = 0;
  int deltaX = 0;
  int deltaY = 0;
  Timer t;

  public static void main(String[] args) {
    movingball ball = new movingball();
    ball.start();

  }

  public void start() {
    ballDrawer drawer = new ballDrawer();
    frame = new JFrame();
    frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);

    JButton up = new JButton("UP");
    JButton down = new JButton("DOWN");
    JButton left = new JButton("LEFT");
    JButton right = new JButton("RIGHT");

    frame.getContentPane().add(BorderLayout.NORTH, up);
    frame.getContentPane().add(BorderLayout.SOUTH, down);
    frame.getContentPane().add(BorderLayout.WEST, left);
    frame.getContentPane().add(BorderLayout.EAST, right);
    frame.getContentPane().add(BorderLayout.CENTER, drawer);
    // setting up the action listener:

    up.addActionListener(new movingup());
    down.addActionListener(new movingdown());
    left.addActionListener(new movingleft());
    right.addActionListener(new movingright());
    frame.setSize(300, 300);
    frame.setVisible(true);
    
    t = new Timer(TIMER_DELAY, new ActionListener() {
      @Override
      public void actionPerformed(ActionEvent e) {
        if(deltaX == 1 && x == frame.getWidth() - CIRCLE_WIDTH) deltaX = 0;
        if(deltaY == 1 && y == frame.getHeight() - CIRCLE_HEIGHT) deltaY = 0;
        if(deltaX == -1 && x == 0) deltaX = 0;
        if(deltaY == -1 && y == 0) deltaY = 0;
        x += deltaX;
        y += deltaY;
        if(deltaX != 0 || deltaY != 0) {
          frame.repaint();
        }
      }
    });
    t.start();

  }// start method end

  // here we have the listeners

  class movingup implements ActionListener {
    @Override
    public void actionPerformed(ActionEvent e) {
      deltaY = -1;
    }
  }// moving up end

  class movingdown implements ActionListener {
    @Override
    public void actionPerformed(ActionEvent e) {
      deltaY = 1;
    }
  }// movingdown end

  class movingleft implements ActionListener {
    @Override
    public void actionPerformed(ActionEvent e) {
      deltaX = -1;
    }
  }// moving left end

  class movingright implements ActionListener {
    @Override
    public void actionPerformed(ActionEvent e) {
      deltaX = 1;
    }
  }// moving right end

  class ballDrawer extends JPanel {
    @Override
    public void paintComponent(Graphics g) {
      super.paintComponent(g);
      g.setColor(Color.white);
      g.fillRect(0, 0, this.getWidth(), this.getHeight());

      g.setColor(Color.black);
      g.fillOval(x, y, CIRCLE_WIDTH, CIRCLE_HEIGHT);

    }// PaintComponents End

  }// ball drawer class end

}// main class endreading: http://stackoverflow.com/q/15511282/1768232

Answer 3

The for loops are causing a lot of lag. Try restructuring your code like this.

public class MovingBall{
    String direction;
    int x=0;
    int y=0;

    public static void main(String[] args){
        //create your frame and buttons...
        upButton.addActionListener(new ActionListener{
            direction = "up";
        });
        //repeat this actionListener to the other buttons
        while (true){
            try{Thread.sleep(50);}catch(Exception e){e.printStackTrace();}
            drawer.repaint();
        }
    }
    public class Drawer extends JPanel{
        public void paint(Graphics g){
            if(direction.equals("up")) {
                y = y - 5;
            }
            //repeat the if statement with the other directions
            g.setColor(Color.white);
            g.fillRect(0,0,frame.getWidth(),frame.getHeight());
            g.setColor(Color.black);
            g.fillOval(x,y,100,100);
        }
    }
}