views:

80

answers:

3

There should be an error/omission in the next code but I can't see where mostly because I've never used threads. Can someone please find what I'm missing?

import java.awt.BorderLayout;
import java.awt.Button;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.util.Timer;
import java.util.TimerTask;

import javax.swing.JDialog;
import javax.swing.JLabel;

public class JavaSwingTest extends JDialog {
    private JLabel m_countLabel;

    private Timer m_timer = new Timer();

    private class IncrementCountTask extends TimerTask {
        @Override
        public void run() {
            m_countLabel.setText(Long.toString(System.currentTimeMillis() 
/ 1000));
        }
    }

    private JavaSwingTest() {
        createUI();

        m_timer.schedule(new IncrementCountTask(), 1000, 1000);
    }

    private void createUI() {
        Button button1 = new Button("Action1");
        button1.addActionListener(new ActionListener() {
            @Override
            public void actionPerformed(ActionEvent arg0) {
                doLongOperation();
            }

        });
        add(button1, BorderLayout.NORTH);

        m_countLabel = new JLabel(Long.toString(System.currentTimeMillis() 
/ 1000));
        add(m_countLabel, BorderLayout.CENTER);
    }

    /**
     * Simulates an operation that takes time to complete.
     */
    private void doLongOperation() {
        try {
            Thread.sleep(3000);
        } catch (InterruptedException e) {
            // ignored for this test
        }
    }

    /**
     * @param args
     */
    public static void main(String[] args) {
        new JavaSwingTest().setVisible(true);
    }
}
+1  A: 

Swing, like most UI toolkits, is not thread-safe. Your setText() call should be forwarded to the event thread.

To initiate GUI work from the timer thread, use SwingUtilities.invokeLater() or invokeAndWait().

Andy Thomas-Cramer
Great answer, thanks.
den-javamaniac
+1  A: 

As the others have said, setText() should be called from the Swing Event Dispatch Thread since it is not threadsafe. The simplest way for you to fix this is to use javax.swing.Timer (which calls the action already on the EDT), not java.util.Timer.

Mark Peters
+1  A: 

Using a SwingWorker would allow your doLongOperation() to execute without blocking the EDT, while also offering a convenient way to periodically update the GUI. Theres a complete example here.

trashgod