TImerTask on paintComponent

Question

I have used a TimerTask to add a timer on a loop on one of my classes which is called by the paint component class. Basically there are white circles on the page already painted, there is then a for loop which reads a value from an array and depending on the range of the value the circle will change colour. each circle should represent the next value in the array. but its not working. I am a basic programmer and really don't understand whats going wrong. I would really appreciate if someone could give me a hand. here is my code so far:

public DoThePaint() {



    String fileName;
    fileName = "/Users/Desktop/test2.txt";
    read = new Reader(fileName);
    read.displayArrayList();

    panel = new JPanel();
    newImage = new ImageIcon(this.getClass().getResource("resource/background2T.png")).getImage();

    circlesT = new ArrayList<Shape>();
    circlesT.add(new Ellipse2D.Float(197.0f, 352.0f, 10.0f, 10.0f));
    circlesT.add(new Ellipse2D.Float(247.0f, 307.0f, 10.0f, 10.0f));
    circlesT.add(new Ellipse2D.Float(152.0f, 303.0f, 10.0f, 10.0f));
    circlesT.add(new Ellipse2D.Float(172.0f, 372.0f, 10.0f, 10.0f));
    circlesT.add(new Ellipse2D.Float(223.0f, 378.0f, 10.0f, 10.0f));
    circlesT.add(new Ellipse2D.Float(273.0f, 285.0f, 10.0f, 10.0f));
    circlesT.add(new Ellipse2D.Float(130.0f, 281.0f, 10.0f, 10.0f));
    circlesT.add(new Ellipse2D.Float(148.0f, 393.0f, 10.0f, 10.0f));
    circlesT.add(new Ellipse2D.Float(246.0f, 403.0f, 10.0f, 10.0f));
    circlesT.add(new Ellipse2D.Float(297.0f, 264.0f, 10.0f, 10.0f));
}
public void paintComponent(Graphics g) {

    drawShapes(g, circlesT);

}

public void drawShapes(Graphics g, final ArrayList<Shape> circlesT) {
    final Graphics2D ga = (Graphics2D) g;
    ga.drawImage(newImage, 0, 0, null);



    for (int i = 0; i < circlesT.size(); i++) {
        ga.draw(circlesT.get(i));
        ga.setPaint(Color.white);
        ga.fill(circlesT.get(i));
    }



    Timer timer = new Timer();
    TimerTask t;
    t = new TimerTask() {
        @Override
        public void run() {

            for (int i = 0; i < 10; i++) {


                if (read.temp.get(i) < 31 && read.temp.get(i) > 30) {
                    ga.draw(circlesT.get(i));
                    ga.setPaint(Color.green);
                    ga.fill(circlesT.get(i));
                } else if (read.temp.get(i) < 32 && read.temp.get(i) > 31) {
                    ga.draw(circlesT.get(i));
                    ga.setPaint(Color.red);
                    ga.fill(circlesT.get(i));
                } else if (read.temp.get(i) < 33 && read.temp.get(i) > 32) {
                    ga.draw(circlesT.get(i));
                    ga.setPaint(Color.yellow);
                    ga.fill(circlesT.get(i));
                }

            }
        }
    };
    repaint();
    timer.schedule(t, 0, 5000);

}

Answer 1

Sorry, but this is a hall of horrors you made: within paintComponent() you are creating a new Timer, a new TimerTask and eventually calling repaint().

1. paintComponent(): all you should do is painting operations (drawing images, filling circles, drawing lines, drawing text, ... on the Graphics). That is all you should do.
2. Calling repaint() will eventually trigger paintComponent() to be invoked again--> recreates the timer, the timer task and recalling repaint()-->paintComponent()->repaint()-->... You get the point, you have just created an infinite loop eating up your resources
3. I have no idea of what your Reader class does and what data it holds. I can see two things that I would change: temp should be private and be accessed properly with a getter or some other access method. Your conditions in your loop are very strict, are you sure all values will match one of the 3 conditions (btw, your condition are all false if read.temp.get(i) returns an int or Integer).
4. To fix all these issues, drop the Timer, drop the TimerTask and within your first for-loop lookup directly the color of the circles and draw them properly immediately. If at some point, you want to update the display, then you can call repaint()
5. Btw, your TimerTask is not run on the EDT (Event dispatching thread) but you still access UI-related stuff from within it. That violates the fact that all UI-related tasks should be performed on the EDT (using Swing Timer or SwingUtilities.invokeLater())
6. In your TimerTask you access Graphics2D ga which is the same as the Graphics g provided by paintComponent(): you cannot access that Graphics object outside the scope of the paintComponent method. Graphics are created and disposed and should therefore not be used outside their scope. In general, you should never keep a reference to a Graphics object (nor use the method getGraphics() which may be tempting but source of all kind of bad headaches).

Answer 2

You have a fundamental misunderstanding of how painting works in Swing. No big deal though, everyone has to start somewhere.

It looks like you are wanting to do animation. In that cause your paintComponent method should draw the state of your object at a particular point in time. Any timer that is controlling the timing of the paint should be in another class.

For simple animation a TimerTask is just fine. If you are wanting to animate a game or something needed good performance I recommend the animation class in Chapter 2 of the book Killer Game Programming in Java by Andrew Davison. You can actually read Chapter 2 at Amazon in their Look Inside feature.