Problems with loops in GUI (java)

Question

I'm pretty new to java, and I just started writing this dumb little program as a GUI test. What it's supposed to do is have 12 buttons, set them all to white, continuously make 3 random buttons black, set all buttons back to white again, wait one second and then repeat. The problem is, I can't seem to make it repeat. Every time I try to put a while or for loop around the section of code that makes the random buttons black, it simply won't run. It doesn't give any errors, and the process itself does run, but no window appears. This is the code for the class (minus the import statements):

public class testingness extends JFrame {

JButton one, two, three, four, five, six, seven, eight, nine, ten, eleven,
        twelve;
JPanel panel;

testingness(String title) {

    super(title);
    this.init();
    this.setSize(800, 800);
    this.setLocationRelativeTo(null);
    this.setVisible(true);
}

void init() {
    panel = new JPanel();
    panel.setLayout(new GridLayout(3, 4));
    one = new JButton();
    one.setBackground(Color.white);
    two = new JButton();
    two.setBackground(Color.white);
    three = new JButton();
    three.setBackground(Color.white);
    four = new JButton();
    four.setBackground(Color.white);
    five = new JButton();
    five.setBackground(Color.white);
    six = new JButton();
    six.setBackground(Color.white);
    seven = new JButton();
    seven.setBackground(Color.white);
    eight = new JButton();
    eight.setBackground(Color.white);
    nine = new JButton();
    nine.setBackground(Color.white);
    ten = new JButton();
    ten.setBackground(Color.white);
    eleven = new JButton();
    eleven.setBackground(Color.white);
    twelve = new JButton();
    twelve.setBackground(Color.white);

    panel.add(one);
    panel.add(two);
    panel.add(three);
    panel.add(four);
    panel.add(five);
    panel.add(six);
    panel.add(seven);
    panel.add(eight);
    panel.add(nine);
    panel.add(ten);
    panel.add(eleven);
    panel.add(twelve);

    this.add(panel);
    while (true) {
        randomness();
        try {
            Thread.sleep(1000);
        } catch (InterruptedException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }
    }
}

public void randomness() {

    for (int timesdone = 0; timesdone < 4; timesdone++) {
        panel.update(panel.getGraphics());
        Random r = new Random();
        int rand = r.nextInt(12);

        if (rand == 0) {
            one.setBackground(Color.black);
        } else if (rand == 1) {
            two.setBackground(Color.black);
        } else if (rand == 2) {
            three.setBackground(Color.black);
        } else if (rand == 3) {
            four.setBackground(Color.black);
        } else if (rand == 4) {
            five.setBackground(Color.black);
        } else if (rand == 5) {
            six.setBackground(Color.black);
        } else if (rand == 6) {
            seven.setBackground(Color.black);
        } else if (rand == 7) {
            eight.setBackground(Color.black);
        } else if (rand == 8) {
            nine.setBackground(Color.black);
        } else if (rand == 9) {
            ten.setBackground(Color.black);
        } else if (rand == 10) {
            eleven.setBackground(Color.black);
        } else if (rand == 11) {
            twelve.setBackground(Color.black);
        }
        one.setBackground(Color.white);
        two.setBackground(Color.white);
        three.setBackground(Color.white);
        four.setBackground(Color.white);
        five.setBackground(Color.white);
        six.setBackground(Color.white);
        seven.setBackground(Color.white);
        eight.setBackground(Color.white);
        nine.setBackground(Color.white);
        ten.setBackground(Color.white);
        eleven.setBackground(Color.white);
        twelve.setBackground(Color.white);

    }
  }

}

What am I doing wrong here?

Put in some breakpoints and monitor the behavior of your program. Is it calling the try, is calling all your setbackground calls, maybe it is but just no refreshing the display. I would first confirm the logic. — mwjohnson, Aug 18 '13 at 23:35
Try taking a look at [this example](http://stackoverflow.com/questions/18266087/change-jlabel-background-on-mouserelease-event/18293834#18293834) — MadProgrammer, Aug 19 '13 at 00:52
You should never call `update` or `paint` directory, this `panel.update(panel.getGraphics());` is very bad — MadProgrammer, Aug 19 '13 at 00:52

MadProgrammer · Answer 1 · 2013-08-30T00:08:10.750

Swing is a single threaded environment. It is required that all interactions and modifications to the UI be carried out within the context of the Event Dispatching Thread.

The EDT is responsible for, amongst other things, process repaint requests and incoming events

Any action or operations which block the EDT will prevent it from processing these events and updating the screen.

In your code you are doing this...

while (true) {
    randomness();
    try {
        Thread.sleep(1000);
    } catch (InterruptedException e) {
        // TODO Auto-generated catch block
        e.printStackTrace();
    }
}

This is likely blocking the EDT, preventing it from ever updating the screen.

Equally, you are also doing this...

for (int timesdone = 0; timesdone < 4; timesdone++) {
   /*...*/
}

Which will equally, block the EDT, preventing it from ever updating the screen...

Also, this panel.update(panel.getGraphics()); is a very bad idea. In Swing, you do not control the paint process, this is up to the RepaintManager. You can make requests for updates/repaints, but it's up to the RepaintManager to decide what and when. These requests are then posted to the EDT...which if blocked, will do nothing.

getGraphics can also return null, which is never pretty...

To solve your problem, you need to off load the "wait" time to some kind of background process, which when ready, needs to tell the UI to change it's state accordingly...

You could use a Thread, but that means you become responsible for resync all the calls back to the EDT, which is messy and offers little gain for the over head.

You could use a SwingWorker, but it's not really suited for the task you have at hand

A better solution would be to use a javax.swing.Timer, which will trigger a actionPerformed event repeatedly after a specified delay, as is executed within the context of the EDT.

import java.awt.BorderLayout;
import java.awt.Color;
import java.awt.Dimension;
import java.awt.EventQueue;
import java.awt.GridLayout;
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;
import javax.swing.UIManager;
import javax.swing.UnsupportedLookAndFeelException;

public class Flashy01 {

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

    public Flashy01() {
        EventQueue.invokeLater(new Runnable() {
            @Override
            public void run() {
                try {
                    UIManager.setLookAndFeel(UIManager.getSystemLookAndFeelClassName());
                } catch (ClassNotFoundException | InstantiationException | IllegalAccessException | UnsupportedLookAndFeelException ex) {
                }

                JFrame frame = new JFrame("Testing");
                frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
                frame.setLayout(new BorderLayout());
                frame.add(new FlashPane());
                frame.pack();
                frame.setLocationRelativeTo(null);
                frame.setVisible(true);
            }
        });
    }

    public class FlashPane extends JPanel {

        private final BigButton one, two, three, four, five, six, seven, eight, nine, ten, eleven,
                twelve;
        private final JPanel panel;
        private final Random random;
        private int timesDone;

        public FlashPane() {
            panel = new JPanel();
            panel.setLayout(new GridLayout(3, 4));
            one = new BigButton();
            one.setBackground(Color.white);
            two = new BigButton();
            two.setBackground(Color.white);
            three = new BigButton();
            three.setBackground(Color.white);
            four = new BigButton();
            four.setBackground(Color.white);
            five = new BigButton();
            five.setBackground(Color.white);
            six = new BigButton();
            six.setBackground(Color.white);
            seven = new BigButton();
            seven.setBackground(Color.white);
            eight = new BigButton();
            eight.setBackground(Color.white);
            nine = new BigButton();
            nine.setBackground(Color.white);
            ten = new BigButton();
            ten.setBackground(Color.white);
            eleven = new BigButton();
            eleven.setBackground(Color.white);
            twelve = new BigButton();
            twelve.setBackground(Color.white);

            panel.add(one);
            panel.add(two);
            panel.add(three);
            panel.add(four);
            panel.add(five);
            panel.add(six);
            panel.add(seven);
            panel.add(eight);
            panel.add(nine);
            panel.add(ten);
            panel.add(eleven);
            panel.add(twelve);

            this.add(panel);
            random = new Random();

            Timer timer = new Timer(1000, new ActionListener() {
                @Override
                public void actionPerformed(ActionEvent e) {
                    randomness();
                }
            });
            timer.start();
        }

        public void randomness() {

            int rand = random.nextInt(12);
            timesDone++;

            if (timesDone % 5 != 0) {

                if (rand == 0) {
                    one.setBackground(Color.black);
                } else if (rand == 1) {
                    two.setBackground(Color.black);
                } else if (rand == 2) {
                    three.setBackground(Color.black);
                } else if (rand == 3) {
                    four.setBackground(Color.black);
                } else if (rand == 4) {
                    five.setBackground(Color.black);
                } else if (rand == 5) {
                    six.setBackground(Color.black);
                } else if (rand == 6) {
                    seven.setBackground(Color.black);
                } else if (rand == 7) {
                    eight.setBackground(Color.black);
                } else if (rand == 8) {
                    nine.setBackground(Color.black);
                } else if (rand == 9) {
                    ten.setBackground(Color.black);
                } else if (rand == 10) {
                    eleven.setBackground(Color.black);
                } else if (rand == 11) {
                    twelve.setBackground(Color.black);
                }

            } else {

                one.setBackground(Color.white);
                two.setBackground(Color.white);
                three.setBackground(Color.white);
                four.setBackground(Color.white);
                five.setBackground(Color.white);
                six.setBackground(Color.white);
                seven.setBackground(Color.white);
                eight.setBackground(Color.white);
                nine.setBackground(Color.white);
                ten.setBackground(Color.white);
                eleven.setBackground(Color.white);
                twelve.setBackground(Color.white);
            }
        }
    }

    public class BigButton extends JButton {

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

    }
}

Take a look at...

For more details...

Wow, I didn't know all this about swing. Thanks for sharing, I'll definitely be looking into those topics you mentioned. — EyedJellyfish, Aug 19 '13 at 01:33

score 2 · Answer 2 · answered Aug 18 '13 at 23:37

2

At the end of the randomness() function you are setting all of the buttons to white. You should move that above all the if-elses, instead of after it.

answered Aug 18 '13 at 23:37

Rob K

294
2
10

score 2 · Accepted Answer · answered Aug 18 '13 at 23:43

As Germann Arlington pointed out, the GUI thread is never being allowed to run because your constructor never returns. However, a better means of getting something performed on a regular basis on the GUI thread (which you need to change the attributes of GUI components) is to use a Timer (http://docs.oracle.com/javase/7/docs/api/javax/swing/Timer.html). E.g.:

int delay = 1000; //milliseconds
ActionListener taskPerformer = new ActionListener() {
    public void actionPerformed(ActionEvent evt) {
        randomness();
    }
};
new Timer(delay, taskPerformer).start();

While this is a better answer, the OP is still going to have problems with the `randomness` method... — MadProgrammer, Aug 19 '13 at 01:10

Germann Arlington · Answer 4 · 2013-08-18T23:44:32.263

0

The main problem is that you code runs, but because it is never ending loop you never get to paint the components - try evoking repaint() method in the end of your method inside the loop.

Another point: if you have multiple buttons you will be better off (programmatically) to add ONLY them to a container and loop over this container content.

edited Aug 18 '13 at 23:44

answered Aug 18 '13 at 23:39

Germann Arlington

3,315
2
17
19

Problems with loops in GUI (java)

4 Answers4