JLabel Overlapping Text in GUI

Question

I'm trying to make a clock using as few resources as possible and just relying on my (limited) knowledge of Java. I've come to a road block however. The clock I wrote works, except rather than the text in the jlabel being replaced, it overlaps itself. I've tried fixing this by clearing the value of timeStamp, but it doesn't seem to be working.

public class Clock extends JFrame{
public static void main (String args[]) {       

    Clock gui = new Clock();
    gui.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
    gui.setSize(50,50);
    gui.setVisible(true);
    gui.setTitle("Clock");

    int a = 1;
    while (a == 1){         
        String timeStamp = new SimpleDateFormat("hh:mm:ss a").format(Calendar.getInstance().getTime());         
        JLabel label;
        label = new JLabel();
        label.setText(String.valueOf(timeStamp));
        timeStamp = "";
        gui.add(label);
        label.revalidate();
    }
           }
}

Answer 1

While the code you wrote "works", you're missing some things to make a stable Swing GUI.

1. You must always start a Swing application using the SwingUtilities invokelater method. This puts the creation and the execution of the Swing components on the Event Dispatch thread (EDT).

2. I separated the creation of the GUI from the execution of the GUI. Separation of concerns makes coding each part easier.

3. In the Timer Runnable, I again use the SwingUtilities invokeLater method to make sure that the updating of the JTextField with the time happens on the EDT.

4. I stop the Thread before I exit. Generally, you should stop any threads you start, and not rely on the JVM to clean up for you.

Here's the clock.

And here's the code.

package com.ggl.testing;

import java.awt.Color;
import java.awt.event.WindowAdapter;
import java.awt.event.WindowEvent;
import java.text.SimpleDateFormat;
import java.util.Calendar;
import java.util.Date;

import javax.swing.BorderFactory;
import javax.swing.JFrame;
import javax.swing.JPanel;
import javax.swing.JTextField;
import javax.swing.SwingUtilities;

public class SimpleClock implements Runnable {

    private JFrame frame;

    private JPanel panel;

    private JTextField clockDisplay;

    private Timer timer;

    @Override
    public void run() {
        frame = new JFrame("Clock");
        frame.setDefaultCloseOperation(JFrame.DO_NOTHING_ON_CLOSE);
        frame.addWindowListener(new WindowAdapter() {
            @Override
            public void windowClosing(WindowEvent event) {
                exitProcedure();
            }
        });

        panel = new JPanel();
        panel.setBorder(BorderFactory.createLineBorder(Color.BLACK, 6));

        clockDisplay = new JTextField(12);
        clockDisplay.setEditable(false);
        clockDisplay.setHorizontalAlignment(JTextField.CENTER);

        panel.add(clockDisplay);

        frame.add(panel);
        frame.pack();
        frame.setLocationByPlatform(true);
        frame.setVisible(true);

        timer = new Timer(this);
        new Thread(timer).start();
    }

    public void exitProcedure() {
        timer.setRunning(false);
        frame.dispose();
        System.exit(0);
    }

    public void setText(String text) {
        clockDisplay.setText(text);
    }

    public static void main(String[] args) {
        SwingUtilities.invokeLater(new Clock());
    }

    public class Timer implements Runnable {

        private volatile boolean running;

        private SimpleClock clock;

        private SimpleDateFormat timeFormat;

        public Timer(SimpleClock clock) {
            this.clock = clock;
            this.running = true;
            this.timeFormat = new SimpleDateFormat("h:mm:ss a");
        }

        @Override
        public void run() {
            while (running) {
                displayTime();
                sleep();
            }

        }

        public void displayTime() {
            final Calendar calendar = Calendar.getInstance();
            Date date = calendar.getTime();
            final String s = timeFormat.format(date);
            SwingUtilities.invokeLater(new Runnable() {
                @Override
                public void run() {
                    clock.setText(s);
                }
            });
        }

        public void sleep() {
            try {
                Thread.sleep(200L);
            } catch (InterruptedException e) {
            }
        }

        public synchronized void setRunning(boolean running) {
            this.running = running;
        }

    }

}

Answer 2

Why are you creating a new JLabel in every iteration of the loop? Don't do that. Just create a single label in Clock's constructor.

Also, changing the label's text should be done on the event thread, not the main thread.

Answer 3

You should use a SwingWorker to update the clock. Currently you're doing it on the event dispatch thread and thus interfere with the UI rendering.

Besides that you should reuse the label instead of creating a new one for each timestamp. Currently you're stacking labels on top of each other since gui.add() will just add the new label and won't remove the old ones.

Answer 4

You should not be creating a new JLabel every iteration.

JLabel label = new JLabel();
gui.add(label);
while (a == 1){         
    String timeStamp = new SimpleDateFormat("hh:mm:ss a").format(Calendar.getInstance().getTime()); 
    label.setText(String.valueOf(timeStamp));
    timeStamp = "";
    label.revalidate();
}