Swing timer for display grid

Question

I'm getting the feeling that I have no idea how swing Timer works. I'm still new to the Java GUI API, and the program I'm writing is just to test myself and help me familiarize myself more with its inner workings.

What it's supposed to do is wait until the user presses the Start button, then iterate the display (a grid of white or black JPanels), which displays a simple cellular automata simulation at a 1 second interval, and pauses when the Pause button is pressed (same as the Start button, but changes name). Each cell in the grid is supposed to start with a random color (white/black). What it's instead doing is to pause for a half second or so, then "run" for another half second, then pause, then run, so on and so forth.

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

public class CA_Driver extends JFrame{

    private JPanel gridPanel, buttonPanel;
    private JButton start_pause, pause;

    private static Timer timer;
    private Color black = Color.black;
    private Color white = Color.white;
    static Color[][] currentGrid, newGrid;
    static Cell[][] cellGrid;
    static boolean run, stop;
    static int height = 20, width = 30, state;

    public CA_Driver(){
        stop = false;
        run = false;
        currentGrid = new Color[height][width];
        newGrid = new Color[height][width];
        cellGrid = new Cell[height][width];

        //Initialize grid values
        for (int x = 0; x < currentGrid.length; x++)
            for (int y = 0; y < currentGrid[x].length; y++){
                int z = (int) (Math.random() * 2);
                if (z == 0)
                    currentGrid[x][y] = newGrid[x][y] = white;
                else currentGrid[x][y] = newGrid[x][y] = black;
            }
        //Create grid panel
        gridPanel = new JPanel();
        gridPanel.setLayout(new GridLayout(height,width));
        //Populate grid 
        for (int x = 0; x < newGrid.length; x++)
            for (int y = 0; y < newGrid[x].length; y++){
                cellGrid[x][y] = new Cell(x,y);
                cellGrid[x][y].setBackground(newGrid[x][y]);
                int z = (int) Math.random();
                if (z == 0) cellGrid[x][y].setBackground(black);
                else cellGrid[x][y].setBackground(currentGrid[x][y]);
                gridPanel.add(cellGrid[x][y]);
            }

        //Create buttons
        state = 0;
        start_pause = new JButton();
        start_pause.setText("Start");
        start_pause.addActionListener(new ActionListener(){
            public void actionPerformed(ActionEvent arg0) {
                if (state == 0) {
                    start_pause.setText("Pause");
                    run = true;
                    timer.start();
                    state += 1;
                }

                else {
                    start_pause.setText("Start");
                    run = false;
                    timer.stop();
                    state -= 1;
                }
            }
        });



        buttonPanel = new JPanel(new BorderLayout());
        buttonPanel.add(start_pause, BorderLayout.NORTH);
//      buttonPanel.add(pause, BorderLayout.EAST);

        //Initialize and display frame
        this.add(gridPanel, BorderLayout.NORTH);
        this.add(buttonPanel, BorderLayout.SOUTH);
        this.setDefaultCloseOperation(EXIT_ON_CLOSE);
        //this.setSize(500, 500);
        pack();
        this.setVisible(true);

        //Initialize timer
        timer = new Timer(1000, new ActionListener(){
            public void actionPerformed(ActionEvent arg0) {


                for (int x = 0; x < cellGrid.length; x++)
                    for (int y = 0; y < cellGrid[x].length; y++){
                        cellGrid[x][y].setColor();
                        currentGrid[x][y] = newGrid[x][y];
                    }
                //Display processing for next frame
                for (int x = 0; x < currentGrid.length; x++)
                    for (int y = 0; y < currentGrid[x].length; y++){
                        int b = checkNeighbors(y,x);
                        if (b > 4 || b < 2)
                            newGrid[x][y] = black;
                        else newGrid[x][y] = white;
                    }
                if(!run) timer.stop();
            }
        });
    }

    public static void main(String[] args) {
        new CA_Driver();
    }


    private int checkNeighbors(int w, int h){
        int b = 0;
        //Top Left
        if((w != 0) && (h != 0) && (currentGrid[h - 1][w - 1] == black))
            b++;
        //Top Middle
        if((h != 0) && (currentGrid[h - 1][w] == black))
            b++;
        //Top Right
        if((w != width - 1) && (h != 0) && (currentGrid[h - 1][w + 1] == black))
            b++;
        //Middle Left
        if((w != 0) && (currentGrid[h][w - 1] == black))
            b++;
        //Middle Right
        if((w != width - 1) && (currentGrid[h][w + 1] == black))
            b++;
        //Bottom left
        if((w != 0) && (h != height - 1) && (currentGrid[h + 1][w - 1] == black))
            b++;
        //Bottom Middle
        if((h != height - 1) && (currentGrid[h + 1][w] == black))
            b++;
        //Bottom Right
        if((w != width - 1) && (h != height - 1) && (currentGrid[h + 1][w + 1] == black))
            b++;
        return b;
    }

    private class Cell extends JPanel{
        private Color c;
        private int posx, posy;

        public Cell(int x, int y){
            posx = x;
            posy = y;
        }

        public Point getLocation(){
            return new Point(posx, posy);
        }

        public void setColor(){
            c = newGrid[posx][posy];
            setBackground(c);
        }

        public Dimension getPreferredSize(){
            return new Dimension(10,10);
        }
    }


}

This is the timer section:

timer = new Timer(1000, new ActionListener(){
    public void actionPerformed(ActionEvent arg0) {


        for (int x = 0; x < cellGrid.length; x++)
            for (int y = 0; y < cellGrid[x].length; y++){
                cellGrid[x][y].setColor();
                currentGrid[x][y] = newGrid[x][y];
            }
        //Display processing for next frame
        for (int x = 0; x < currentGrid.length; x++)
            for (int y = 0; y < currentGrid[x].length; y++){
                int b = checkNeighbors(y,x);
                if (b > 4 || b < 2)
                    newGrid[x][y] = black;
                else newGrid[x][y] = white;
            }
        if(!run) timer.stop();
    }
    });

I'm planning on adding more features later to give the user more control over various variables such as the grid size and iteration speed, but I want to get the core functionality of the display working. I'm fairly sure the issue is in how I'm using the Timer class since it's the timing that's broken.

My first question is: Am I using the Timer class right? If so, then what is the issue? If not, how should I be using it?

Update That's a good idea, MadProgrammer, and it's good to know I'm using Timer correctly. I realized that the part where it was "running" was actually how long it took each individual cell to update its color, so really my program is just absurdly slow and inefficient as it is now.

Here's my idea to improve the speed and efficiency. Mainly, I would use the timer delay to process the output of the next iteration, then the next time the timer "fires" I would change a "tick" variable that each cell would use as their signal to change color, as suggested. To accomplish this, I've added a timer to each cell (how good/bad an idea is this?) that kill time for a bit, then, in a blocking while loop, wait to see that the internal "tick" is equivalent to the global "tick" and immediately change color when that happens.

The end result is that it freezes as soon as it starts.

This is the timer I added to the Cell class constructor:

c_timer = new Timer(500, new ActionListener(){
    public void actionPerformed(ActionEvent e){
        c_timer.stop();
        while (c_tick != tick);
        setBackground(currentGrid[posx][posy]);
        c_tick = 1 - c_tick;
        if(run) timer.restart();
    }
});
        c_timer.start();

And this is how I've modified the global timer:

timer = new Timer(1000, new ActionListener(){
    public void actionPerformed(ActionEvent arg0) {
        for (int y = 0; y < height; y++)
            for (int x = 0; x < width; x++)
                currentGrid[y][x] = newGrid[y][x];
        tick = 1 - tick;

        for (int y = 0; y < height; y++)
            for (int x = 0; x < width; x++){
                if (b[y][x] > 6 || b[y][x] < 1) newGrid[y][x] = white;
                else newGrid[y][x] = black;
            }

        for (int y = 0; y < height; y++)
            for (int x = 0; x < width; x++)
                b[y][x] = checkNeighbors(x,y);
        if(!run) timer.stop();
    }
});

Other than these changes, I removed the setColor() method in the Cell class. Can anyone point out the mistake that I'm making?

UPDATE 2

I should have updated earlier, but simply put, I discovered this is entirely the wrong way to do it. Instead of making a panel full of components and changing their backgrounds, you should instead just paint the panel with a grid:

@Override
public void paintComponent(Graphics g){
    super.paintComponent(g);
    for (int h = 0; h < board_size.height; h++){
        for (int w = 0; w < board_size.width; w++){
            try{
                if (grid[h][w] == BLACK)
                    g.setColor(BLACK);
                else g.setColor(WHITE);
                g.fillRect(h * cell_size, w * cell_size, cell_size, cell_size);
            } catch (ConcurrentModificationException cme){}
        }
    }
}

On each timer "tick" you first repaint the grid, then you process the next iteration to be painted on the next tick. Far more efficient, and updates instantly.

My I used a modified JPanel as the main grid component which implements an ActionListener to process every action the user performs on the rest of the gui as well as each timer tick:

public void actionPerformed(ActionEvent e) {

        //Timer tick processing: count surrounding black cells, define next iteration
            //using current rule set, update master grid 
        if (e.getSource().equals(timer)){
        //Processing for each tick
        }

        else if(e.getSource()...
        //Process events dispached by other components in gui
}

Of course, you'd have to set the board panel as the action listener for the timer.

Answer 1

Your usage of the Timer class in the first part of the question indeed looks correct. What is happening with a java.swing.Timer is that the ActionListener is triggered on the Event Dispatch Thread at specific intervals, specified with the delay parameter.

This also means that the code you put in the ActionListener should execute quickly. While your ActionListener code is executing, the UI cannot update as the UI thread (the Event Dispatch Thread) is occupied executing the ActionListener code. This is clearly documented in the javadoc of that class.

Although all Timers perform their waiting using a single, shared thread (created by the first Timer object that executes), the action event handlers for Timers execute on another thread -- the event-dispatching thread. This means that the action handlers for Timers can safely perform operations on Swing components. However, it also means that the handlers must execute quickly to keep the GUI responsive.

This is exactly what you encountered in your first update

new Timer(500, new ActionListener(){
  public void actionPerformed(ActionEvent e){
    //...
    while (c_tick != tick){}
    //...
  }
});

With the while loop here you are blocking the Event Dispatch Thread. The c_tick != tick check will never change as the variables involved are only adjusted on the EDT, and you are blocking it with the loop.

Your second update seems to suggest everything is working now by switching from a panel. There are however two weird looking things:

1. The catch ConcurrentModificationException cme code block. In the code you posted I cannot immediately spot where you would encounter a ConcurrentModificationException. Remember that Swing is single-threaded. All actions which could interact with Swing components should be executed on the EDT, making the chance on encountering a ConcurrentModificationException a lot smaller compared to a multi-threaded application.

2. You stated

   Of course, you'd have to set the board panel as the action listener for the timer

   This seems untrue. Whatever ActionListener attached to the Timer needs to swap the current grid and the next grid, and calculate the next grid. Once the next grid is calculated, it needs to schedule a repaint of the grid panel. Whether or not this ActionListener is an anonymous/inner/separate class or the grid panel itself is irrelevant (at least functionality wise, design wise I would never opt to let the grid panel be a listener).

Side note: when you need to swap the current and new grid you use the following code

for (int y = 0; y < height; y++){
  for (int x = 0; x < width; x++){
    currentGrid[y][x] = newGrid[y][x];
  }
}

If you still have performance problems, you can try using System.arrayCopy which is probably much faster then looping over the array manually.