Swing - Thread.sleep() stop JTextField.setText() working

Question

Possible Duplicate:
using sleep() for a single thread

I'm having issues with JTextField.setText() when using Thread.sleep(). This is for a basic calculator I'm making. When the input in the input field is not of the correct format I want "INPUT ERROR" to appear in the output field for 5 seconds and then for it to be cleared. The setText() method did work when I just set the text once to "INPUT ERROR" and by printing out the text in between I found it does work with both that and the setText("") one after the other. The problem arises when I put the Thread.sleep() between them. Here's a SSCCE version of the code:

import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.util.regex.Pattern;
import javax.swing.*;

public class Calc {
    static Calc calc = new Calc();

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

    public class GUI implements ActionListener {

        private JButton equals;

        private JTextField inputField, outputField;

        public GUI() {
            createFrame();
        }

        public void createFrame() {
            JFrame baseFrame = new JFrame("Calculator");
            baseFrame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
            JPanel contentPane = new JPanel();
            BoxLayout layout = new BoxLayout(contentPane, BoxLayout.Y_AXIS);
            contentPane.setLayout(layout);
            baseFrame.setContentPane(contentPane);
            baseFrame.setSize(320, 100);

            equals = new JButton("=");
            equals.addActionListener(this);

            inputField = new JTextField(16);
            inputField.setHorizontalAlignment(JTextField.TRAILING);
            outputField = new JTextField(16);
            outputField.setHorizontalAlignment(JTextField.TRAILING);
            outputField.setEditable(false);

            contentPane.add(inputField);
            contentPane.add(outputField);
            contentPane.add(equals);

            contentPane.getRootPane().setDefaultButton(equals);
            baseFrame.setResizable(false);
            baseFrame.setLocation(100, 100);

            baseFrame.setVisible(true);
        }

        /**
         * When an action event takes place, the source is identified and the
         * appropriate action is taken.
         */

        @Override
        public void actionPerformed(ActionEvent e) {
            if (e.getSource() == equals) {
                inputField.setText(inputField.getText().replaceAll("\\s", ""));
                String text = inputField.getText();
                System.out.println(text);
                Pattern equationPattern = Pattern.compile("[\\d(][\\d-+*/()]+[)\\d]");
                boolean match = equationPattern.matcher(text).matches();
                System.out.println(match);
                if (match) {
                    // Another class calculates
                } else {
                    try {
                        outputField.setText("INPUT ERROR"); // This doesn't appear
                        Thread.sleep(5000);
                        outputField.setText("");
                    } catch (InterruptedException e1) {
                    }
                }
            }
        }
    }
}

I'm not actually using a nested class but I wanted it to be able to be contained in one class for you. Sorry about how the GUI looks but again this was to cut down the code. The the important section (if (e.getSource() == equals)) remains unchanged from my code. The simplest way to give an incorrect input is to use letters.

try putting a `System.out.println()` there to see if it ever gets to that point — tckmn, Jan 01 '13 at 00:11
@Doorknob I had done that in my own code and it _was_ reaching that point. — PElshen, Jan 01 '13 at 13:20

aly · Accepted Answer · 2013-01-01T16:51:21.130

13

When you use Thread.sleep() you're doing it on the main thread. This freezes the gui for five seconds then it updates the outputField. When that happens, it uses the last set text which is blank.

It's much better to use Swing Timers and here's an example that does what you're trying to accomplish:

if (match) {
    // Another class calculates
} else {
    outputField.setText("INPUT ERROR");
    ActionListener listener = new ActionListener(){
        public void actionPerformed(ActionEvent event){
            outputField.setText("");
        }
    };
    Timer timer = new Timer(5000, listener);
    timer.setRepeats(false);
    timer.start();
}

edited Jan 01 '13 at 16:51

answered Jan 01 '13 at 00:25

aly

523
2
7

1

Use `invokeLater()` in `TimerTask` or, better, `javax.swing.Timer`. – trashgod Jan 01 '13 at 00:43
2

@trashgod Good idea, I changed the code accordingly – aly Jan 01 '13 at 00:53
Works perfectly on my complete code, thank you @aly – PElshen Jan 01 '13 at 13:14
I think this will add a new recurring timer that will run forever on a 5000ms cycle for each and every input error. – clstrfsck Jan 01 '13 at 14:27
@msandiford You're right! Fixed again – aly Jan 01 '13 at 16:51

Philip Whitehouse · Answer 2 · 2013-01-01T11:47:29.513

8

You're doing a Thread.sleep() in the Swing main thread. This is NOT good practice. You need to use a SwingWorker thread at best.

What's happening is that it's running the first line, hitting Thread.sleep().

This prevents the (main) EDT thread from doing any of the repaints (as well as preventing the next line executing).

You should use a javax.swing.Timer to setup the delayed reaction and not put sleep() calls in the main thread.

edited Jan 01 '13 at 11:47

answered Jan 01 '13 at 00:17

Philip Whitehouse

4,293
3
23
36

You are right that thread sleep should not be used here, but I don't think the sleep is being interrupted. 1+ – Hovercraft Full Of Eels Jan 01 '13 at 01:11
That's the only reason it wouldn't proceed to execute the second set text. It's not clear what the actual failed outcome was. In any case it's perfectly plausible that it could be interrupted, thus producing an error. – Philip Whitehouse Jan 01 '13 at 01:15
3

If you modify his code and print out the stacktrace for the InterruptedException, you'll see that it never occurs. Instead `outputField.setText("INPUT ERROR");` actually does get called, `Thread.sleep(5000);` gets called, and then `outputField.setText("");` after the 5 second delay, but you never see the "INPUT ERROR" in the JTextField because the `Thread.sleep(...)` puts the Swing Event Dispatch Thread or **EDT** to sleep, preventing it from doing any of its actions which include painting the GUI including the text field's contents. – Hovercraft Full Of Eels Jan 01 '13 at 03:36
Thanks to both of you for the explanation, I understand now why it was happening - my understanding of threads is limited but this has helped! – PElshen Jan 01 '13 at 13:12

clstrfsck · Answer 3 · 2013-01-01T01:04:10.600

8

As Philip Whitehouse states in his answer, you are blocking the swing Event Dispatch Thread with the Thread.sleep(...) call.

Given that you've taken the time to set up an ActionListener already, it would probably be easiest to use a javax.swing.Timer to control clearing the text. To do this, you could add a field to your GUI class:

    private Timer clearTimer = new Timer(5000, this);

In the constructor for GUI, turn off the repeats feature, as you really only need a one-shot:

    public GUI() {
        clearTimer.setRepeats(false);
        createFrame();
    }

Then, actionPerformed can be modified to use this to start the timer/clear the field:

    public void actionPerformed(ActionEvent e) {
        if (e.getSource() == equals) {
            inputField.setText(inputField.getText().replaceAll("\\s", ""));
            String text = inputField.getText();
            System.out.println(text);
            Pattern equationPattern = Pattern.compile("[\\d(][\\d-+*/()]+[)\\d]");
            boolean match = equationPattern.matcher(text).matches();
            System.out.println(match);
            if (match) {
                // Another class calculates
            } else {
                clearTimer.restart();
                outputField.setText("INPUT ERROR"); // This doesn't appear
            }
        } else if (e.getSource() == clearTimer) {
            outputField.setText("");
        }
    }

edited Jan 01 '13 at 01:04

answered Jan 01 '13 at 00:52

clstrfsck

14,715
4
44
59

You are correct to recommend use of a Swing Timer, but my preference is not to ask an ActionListener to do too much as it can be a mess to debug and maintain. Why not instead just use anonymous inner classes for the listeners? 1+ for your good advice. – Hovercraft Full Of Eels Jan 01 '13 at 03:51
This solution worked on my example but not on my original code (it didn't disappear), perhaps @HovercraftFullOfEels's point is true in that in more complex code problems arise. aly's solution does work. – PElshen Jan 01 '13 at 13:06
@PaddyProgrammer: no it would work in your original code fine if implemented correctly. Your problem was undoubtably in your implementation of his recommendation. Note that whenever you get code in answers here, the code is never meant as drop in solutions to your problems but rather example code whose ideas you can use for the code solution that you ultimately create yourself with these ideas. – Hovercraft Full Of Eels Jan 01 '13 at 13:09
@HovercraftFullOfEels I understand and although I didn't just drop in the solution, I probably didn't implement it correctly, but based on what you said it may be not have been the best solution anyway. – PElshen Jan 01 '13 at 13:18
@PaddyProgrammer: all the solutions you've received are essentially the same solution, so it is just as good as any other. I just had a minor quibble with one of his implementation details (and actually your posted implementation as well). In general a GUI class should not implement a listener interface. You should never use `this` as an ActionListener or MouseListener or any such listener. You're much better off using anonymous inner classes for your listeners. – Hovercraft Full Of Eels Jan 01 '13 at 13:21
1

FWIW, I agree with what Mr. Hovercraft is saying in that the code could use some restructuring/application of best practices. My intention was to provide a correct solution that required minimal code changes. – clstrfsck Jan 01 '13 at 14:26

Swing - Thread.sleep() stop JTextField.setText() working

3 Answers3

Linked

Related