JFrame repaint(); How to update the JFrame with Thread info?

Question

I was trying to set up a simple time ticker on a JFrame. I am able to display the initial value of counter, but any subsequent change doesn't update on the JFrame. Any nudge towards an answer to what I am doing wrong would be appreciated. I think it is a problem of repaint() not getting called, but I either end up with errors, or nothing if I try putting it in.

package com.game.ryan;
import java.awt.Dimension;
import javax.swing.JFrame;
class Screen extends JFrame{


private Dimension d = new Dimension(800,600);
private JFrame f;

public Screen(){
    f = new JFrame();
    f.setIgnoreRepaint(false);
    f.setResizable(false);
    f.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
    f.setMinimumSize(d);
    f.setLocationRelativeTo(null);
    f.add(new MyPanel());
    f.pack();
    f.setVisible(true);
}

public static void main(String[] args){
    Screen s = new Screen();
}

}

I also have:

package com.game.ryan;

import java.awt.Graphics;

import javax.swing.JPanel;

public class MyPanel extends JPanel{

private OneThread ot = new OneThread();
private int counter = ot.getThreadCounter();

public MyPanel(){
    Thread t1 = new Thread(new OneThread());
    t1.start();

}

public void paintComponent(Graphics g){
    g.drawString("TIME: ", 10, 20);
    g.drawString(Integer.toString(counter), 50, 20);        
}
}

and finally

package com.game.ryan;

public class OneThread implements Runnable{

private int counter = 45;

public OneThread(){

}

@Override
public void run() {

    for(int x = 0; x >= 0; x++){
        try{
            Thread.sleep(1000);
            counter++;
            x++;
            System.out.println(counter);

        }catch(Exception e){
            e.printStackTrace();
        }
    }       
}

public int getThreadCounter(){
    return counter;
}
}

I am getting an increasing counter on the console so I guess that part is working correctly.

Expected result was for the counter to display correctly in the JFrame (updating every 1000ms).

Answer 1

I don't see anywhere you tell the UI it should update itself.

You're also going a long way out of your way to replicate what is already available within the APIs.

Swing and threads need careful consideration. Swing uses a single threaded model for managing all the updates to the UI. It is expected that all iterations with the UI will be done within the context of the thread (AKA The Event Dispatching Thread).

This means any time you want to create or update the UI from any other thread, you need to synchronize the calls back to the EDT.

While there are a number of ways to achieve this, the simplest in your case in the use of javax.swing.Timer

import java.awt.BorderLayout;
import java.awt.EventQueue;
import java.awt.GridBagLayout;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.text.DateFormat;
import java.text.SimpleDateFormat;
import java.util.Date;
import javax.swing.JFrame;
import javax.swing.JLabel;
import javax.swing.Timer;
import javax.swing.UIManager;
import javax.swing.UnsupportedLookAndFeelException;

public class LabelClock {

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

    protected static final DateFormat DATE_FORMAT = new SimpleDateFormat("HH:mm.ss");
    private JLabel clock;

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

                clock = new JLabel();
                tick();

                JFrame frame = new JFrame("Testing");
                frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
                frame.setLayout(new GridBagLayout());
                frame.add(clock);
                frame.pack();
                frame.setLocationRelativeTo(null);
                frame.setVisible(true);

                Timer timer = new Timer(500, new ActionListener() {
                    @Override
                    public void actionPerformed(ActionEvent e) {
                        tick();
                    }
                });
                timer.setRepeats(true);
                timer.setCoalesce(true);
                timer.start();
            }
        });
    }

    protected void tick() {
        clock.setText(DATE_FORMAT.format(new Date()));
    }

}

Answer 2

You may wish to re-think your design:

• A primitive variable holds a value and that's it. If you assign the value held by the primitive to another variable, changing the original variable later will have no effect on the value held by the other variable. For instance, nothing in your code changes value held by the counter variable held by the JPanel.
• Note that even if your plan were successful, you have two completely independent OneThread objects, and that changing the state of one will have no effect on the other.
• Better to have your GUI listen for changes to a variable and then have your thread change the state of the variable and then notify all listeners of this change. A PropertyChangeListener could work well for this.
• Note that a Swing Timer would be much easier to implement then a background thread.