Multithread timer not working correctly

Question

Again, please help me! In the following code, I want to get started by pushing a timer thread at the push of a button and typing it in a Label. Each press of the button should start a new thread and mark it on each label. But unfortunately, the same timer is written for each label. Can you help us get it right? If you can tell me the mistake, what do I mean?

public class TimerThreads implements ActionListener{

    JFrame jFrame = new JFrame();
    JLabel[] labels;
    int second = 0;
    int minute = 0;
    String s = "";
    String m = "";
    int l = 0;


    public TimerThreads(){
        JLabel one = new JLabel();
        JLabel two = new JLabel();
        JLabel three = new JLabel();
        JLabel four = new JLabel();
        labels = new JLabel[]{one, two, three, four};

        jFrame.setLayout(new GridLayout(0, 2, 5, 5));
        jFrame.setDefaultCloseOperation(JFrame.DISPOSE_ON_CLOSE);

        JButton start = new JButton("Start");
        start.addActionListener(this);

        JButton stop = new JButton("Stop");
        stop.addActionListener(this);

        jFrame.add(start);
        jFrame.add(stop);
        jFrame.setVisible(true);
        jFrame.pack();
    }

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

    @Override
    public void actionPerformed(ActionEvent e) {
        String select = e.getActionCommand();
        switch(select){
            case "Start":
                jFrame.add(labels[l]);
                jFrame.revalidate();
                jFrame.repaint();
                TimerThread t = new TimerThread(labels[l]);
                t.start();
                l++;
                break;
            case "Stop":
                //
                break;
        }
    }

    class TimerThread extends Thread{
        JLabel jLabel;

        public TimerThread(JLabel jLabel) {
            this.jLabel = jLabel;
        }

        @Override
        public void run() {
            Timer timer = new Timer();
            timer.scheduleAtFixedRate(new TimerTask() {
                @Override
                public void run() {
                    second++;
                    if(String.valueOf(second).length() == 1){
                        s = "0";
                    }
                    else{
                        s = "";
                    }
                    if(second == 60){
                        second = 0;
                        s = "0";
                        minute++;
                    }

                    if(String.valueOf(minute).length() == 1){
                        m = "0";
                    }
                    jLabel.setText(m + String.valueOf(minute) + ":" + s + String.valueOf(second));
                }
            },0, 1000);
        }
    }
}

Answer 1

The cause of your bug is here:

public class TimerThreads implements ActionListener {

    JFrame jFrame = new JFrame();
    JLabel[] labels;

    // ***** these fields below
    int second = 0;     
    int minute = 0;
    String s = "";
    String m = "";
    // ***** these fields above

    int l = 0;

The four fields are instance fields of the class and are shared by every TimerTask instance that you create, and so all will show the same exact time.

The solution is to make these fields local to the nested class:

public class TimerThreads implements ActionListener {

    JFrame jFrame = new JFrame();
    JLabel[] labels;
    // int second = 0;
    // int minute = 0;
    // String s = "";
    // String m = "";
    int l = 0;

    public TimerThreads() {
        //.....
    }

    // ....


    class TimerThread extends Thread {
        JLabel jLabel;

        public TimerThread(JLabel jLabel) {
            this.jLabel = jLabel;
        }

        @Override
        public void run() {
            java.util.Timer timer = new java.util.Timer();
            timer.scheduleAtFixedRate(new TimerTask() {

                // ***** add these fields here
                int second = 0;
                int minute = 0;
                String s = "";
                String m = "";

---

Having said this, you've got risky code since you're using the wrong Timer, a java.util.Timer, when you should be using a Swing Timer or javax.swing.Timer. This is very important since the latter Timer works well with the Swing event model and prevents thread clashes. Please check out the Swing Timer Tutorial

Other problems: using a fixed size array, you risk index out of bounds exception if the user wants more than 4 threads running. Use an ArrayList instead.