Painting inside Swing Timer not working

Question

I never worked with Timers before so my problem is probably stupid one really. My program draws a circle which is red and after random seconds the circle should change its color to green. I just made a swing timer as you can see below in the code. And it enters actionPerformed() method but it doesn't change color. Could you help me somehow fix my problem with changing colors?

My code:

package igrica;
import java.awt.BorderLayout;
import java.awt.Color;
import java.awt.Graphics;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.util.Random;

import javax.swing.JButton;
import javax.swing.JFrame;
import javax.swing.JPanel;
import javax.swing.Timer;


public class ChangingCircle implements ActionListener{

JFrame frame;

Timer timer;
Random r;

public static void main(String[] args) {
    ChangingCircle gui = new ChangingCircle();
    gui.go();
}

public void go() {
    frame = new JFrame();
    frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);

    MyPanel panel = new MyPanel();

    frame.getContentPane().add(BorderLayout.CENTER, panel);
    frame.setSize(300, 300);
    frame.setVisible(true);
}   

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

class MyPanel extends JPanel {
    public void paintComponent(Graphics g) {


        g.setColor(Color.red);
        g.fillOval(100, 100, 100, 100);

        Random r = new Random();

        Timer timer = new Timer(r.nextInt(5000) + 1000, new ActionListener() {
            public void actionPerformed(ActionEvent ev) {
                System.out.println("Timer out");
                g.setColor(Color.green);
                g.fillOval(100, 100, 100, 100);
            } 
        });
        timer.start();
    }
}
}

Answer 1

The timer works asynchronously and paintComponent finishes before finishing the work of timer.

Answer 2

There's quite the mess in your code. Try this:

public class ChangingCircle {

    Color color = Color.RED;
    MyPanel panel = new MyPanel();

    public static void main(String[] args) {

        SwingUtilities.invokeLater(() -> {
            ChangingCircle gui = new ChangingCircle();
            gui.go();
        });
    }

    public void go() {

        JFrame frame = new JFrame();
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);

        frame.getContentPane().add(panel, BorderLayout.CENTER);
        frame.pack();
        frame.setVisible(true);

        Random r = new Random();
        Timer timer = new Timer(r.nextInt(5000) + 1000, new ActionListener() {

            public void actionPerformed(ActionEvent ev) {

                System.out.println("Timer");
                color = Color.GREEN;
                panel.repaint();
            }
        });
        timer.setRepeats(false);
        timer.start();
    }

    class MyPanel extends JPanel {

        private int size = 100, loc = 100;

        @Override
        public void paintComponent(Graphics g) {

            super.paintComponent(g);
            g.setColor(color);
            g.fillOval(loc, loc, size, size);
        }

        @Override
        public Dimension getPreferredSize() {

            return new Dimension(size + loc, size + loc);
        }
    }
}

The idea is that the timer only changes the property of the shape to be drawn and then calls repaint() to reflect the change. The paintComponent is called whenever it is needed, even in quick succession and should return quickly.

Specific Notes:

• Start Swing from the EDT.
• Create and start the timer from outside of paintComponent since it is called many times and that will create and start many timers.
• You should probably set the timer not to repeat.
• Call super.paintComponent(g); as the first thing inside paintComponent.
• You seem to have an ActionListener that does nothing.

General tips:

• Use the @Override annotation when applicable.
• Call pack() on the frame instead of setting its size manually and @Override the getPreferredSize method of the component you paint on. Return a meaningful size based on what you draw.
• Use add(component, location) and not the other way around (deprecated).
• Don't use fields when local variables will do (Random r for example).
• Use uppercase constant names (Color.RED instead of Color.red).

Answer 3

Don't initiate a Timer from within a paintComponent method. This method should be for painting and painting only. Instead start the Timer in your constructor and within your Timer's actionPerromed and call repaint(), change the state of a field of the class, and use that information within the paintComponent use that field to draw any new information.

e.g.

import java.awt.BorderLayout;
import java.awt.Color;
import java.awt.Graphics;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.util.Random;
import javax.swing.JButton;
import javax.swing.JFrame;
import javax.swing.JPanel;
import javax.swing.Timer;

public class ChangingCircle {
    JFrame frame;

    public static void main(String[] args) {
        ChangingCircle gui = new ChangingCircle();
        gui.go();
    }

    public void go() {
        frame = new JFrame();
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);

        MyPanel panel = new MyPanel();

        frame.getContentPane().add(BorderLayout.CENTER, panel);
        frame.setSize(300, 300);
        frame.setVisible(true);
    }   

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

    class MyPanel extends JPanel {
        private Random r = new Random();
        private boolean draw = false;

        public MyPanel() {
            Timer timer = new Timer(r.nextInt(5000) + 1000, new ActionListener() {
                public void actionPerformed(ActionEvent ev) {
                    draw = true;
                    repaint();
                } 
            });
            timer.setRepeats(false);
            timer.start();
        }
        public void paintComponent(Graphics g) {
            super.paintComponent(g);
            if (draw) {
                g.setColor(Color.red);
                g.fillOval(100, 100, 100, 100);
            }
        }
    }
}

Also, don't forget to call the super's paintComponent method from within your override.

If you need to change colors, give the JPanel a Color field, say called color and change it's value from within the Timer, and then call repaint(). Again within paintComponent, use the value of that field to draw the oval with. Also in this situation, the Timer should repeat, so get rid of timer.setRepeats(false) in that situation.