CODEWITHSUNDEEP
javamultithreadingswing
GAURANG VYAS
GAURANG VYAS

Reputation: 689

Unknown behaviour with Java Thread

I am trying to implement a Java Stop Watch in which I have used a Thread to continuously moniter time elapsed. But i have encountered a problem in my code. My Java Code is - 

  import java.awt.FlowLayout;
  import java.awt.GridBagConstraints;
  import java.awt.GridBagLayout;
  import java.awt.event.ActionEvent;
  import java.awt.event.ActionListener;
  import java.util.Calendar;

  import javax.swing.*;

  public class Prac2_StopWatch implements ActionListener{
      JFrame Time;
      JLabel TimeLabel;
      JButton StartStop;
      JButton Reset;
      StopWatch stThread;
      String prefix="<html><h1><body>";
      String suffix="</h1></body></html>";
      StopWatch myControl;

      Prac2_StopWatch()
     {
         Time = new JFrame();
         Time.setSize(275,275);
         Time.setTitle("Stop Watch");
         Time.setLayout(new GridBagLayout());
         GridBagConstraints gridConst = new GridBagConstraints();

         TimeLabel = new JLabel();
         TimeLabel.setText(prefix+"0:0:0"+suffix);

         myControl = new StopWatch(TimeLabel);
         myControl.start();

         StartStop = new JButton();
         Reset = new JButton();

         StartStop.setActionCommand("Start");
         StartStop.setText("Start");
         Reset.setText("Reset");

         StartStop.addActionListener(this);
         Reset.addActionListener(this);

         gridConst.gridx=0;
         gridConst.gridy=0;
         Time.add(StartStop,gridConst);

         gridConst.gridx=1;
         gridConst.gridy=0;
         Time.add(Reset,gridConst);

         gridConst.gridx=0;
         gridConst.gridy=1;
         Time.add(TimeLabel,gridConst);

         Time.setVisible(true);



      }
      public void actionPerformed(ActionEvent evt)
     {
         if(evt.getActionCommand()=="Start")
         {
            StartStop.setActionCommand("Stop");
            StartStop.setText("Stop");
            if(myControl.curMil==-1)
                 myControl.curMil=Calendar.getInstance().getTimeInMillis();
            else 
                myControl.curMil=Calendar.getInstance().getTimeInMillis()-      myControl.diff;
                myControl.count=true;

             }
            else if(evt.getActionCommand()=="Stop")
           {
              StartStop.setActionCommand("Start");
              StartStop.setText("Start");
              myControl.count=false;

           }
           else if(evt.getActionCommand()=="Reset")
           {
               StartStop.setActionCommand("Start");
               StartStop.setText("Start");
               myControl.count=false;
               myControl.curMil=-1;
               TimeLabel.setText(prefix+"0:0:0"+suffix);

           }
        }

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

        }
   }

   class StopWatch extends Thread
   {
        long curMil;
        long diff;
        JLabel TimeLabel;
        boolean count;
        String prefix="<html><body><h1>";
        String suffix="</h1></body></html>";

        StopWatch(JLabel TimeLabel)
        {
           this.TimeLabel = TimeLabel;
           this.count=false;
           this.curMil=-1;
         }
       public void run()
       {
           while(true){
           System.out.print("Commenting this line will make stop watch useless");
           if(count){
                diff= Calendar.getInstance().getTimeInMillis() - curMil;
                TimeLabel.setText(prefix+((diff/60000)%60)+" : "+((diff/1000)%60)+" : "+(diff%1000)+suffix);
                try {
                    Thread.sleep(1);
                } catch (InterruptedException e) {
                   e.printStackTrace();
              }
           }
        }
     }
 }

Now the problem is that in the run method of StopWatch class when I comment the statement - 

System.out.println("Commenting this line will make stop watch useless");

in line 123.
My stop watch stops working i.e. the clock always displays "0:0:0" even if press 'Start', 'Stop' or 'Reset'. Moreover, replacing the line 123 with any other statement other than 'System.out.println()' has the same issue.

Upvotes: 0

Views: 92

Answers (2)

Yousaf
Yousaf

Reputation: 29282

Move

if(count){
  diff= Calendar.getInstance().getTimeInMillis() - curMil;
  TimeLabel.setText(prefix+((diff/60000)%60)+":"+((diff/1000)%60)+" : "+(diff%1000)+suffix);
}

inside try-catch block.

Side notes:

  1. Your program doesn't stops when JFrame window is closed. Use

    Time.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);

  2. Proper way to compare Strings in java is 

    if("string1".equals("string2")) {
    //do stuff
}

Upvotes: 3

Hovercraft Full Of Eels
Hovercraft Full Of Eels

Reputation: 285405

Two main problems that I see:

  • You've got a tight loop, one with a 1 msec pause that will hog the processor [Edit: this is actually OK]
  • You're making Swing calls, here setText(...) from your background thread, something that you shouldn't be doing, and that with the tight loop can completely freeze your Swing GUI.
  • You're sharing the count boolean variable between threads, and so it must be declared volatile so that its state will be the same in all threads.

If you need to make Swing state changes from within a background thread, use a SwingWorker and make the changes using the publish/process pair.

Also, don't compare Strings using == or !=. Use the equals(...) or the equalsIgnoreCase(...) method instead. Understand that == checks if the two object references are the same which is not what you're interested in. The methods on the other hand check if the two Strings have the same characters in the same order, and that's what matters here. So instead of

else if (evt.getActionCommand() == "Stop") {

do

else if (evt.getActionCommand().equalsIgnoreCase("Stop")) {

e.g.,

import java.awt.GridBagConstraints;
import java.awt.GridBagLayout;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.util.Calendar;
import javax.swing.*;

public class Prac2_StopWatch implements ActionListener {
    JFrame time;
    JLabel timeLabel;
    JButton startStop;
    JButton reset;
    StopWatch stThread;
    String prefix = "<html><h1><body>";
    String suffix = "</h1></body></html>";
    StopWatch myControl;

    Prac2_StopWatch() {
        time = new JFrame();

        time.setSize(275, 275);
        time.setTitle("Stop Watch");
        time.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        time.setLayout(new GridBagLayout());
        GridBagConstraints gridConst = new GridBagConstraints();

        timeLabel = new JLabel();
        timeLabel.setText(prefix + "00:00:000" + suffix);

        myControl = new StopWatch(timeLabel);
        new Thread(myControl).start(); // StopWatch should implement Runnable
        // myControl.start();

        startStop = new JButton();
        reset = new JButton();

        startStop.setActionCommand("Start");
        startStop.setText("Start");
        reset.setText("Reset");

        startStop.addActionListener(this);
        reset.addActionListener(this);

        gridConst.gridx = 0;
        gridConst.gridy = 0;
        time.add(startStop, gridConst);

        gridConst.gridx = 1;
        gridConst.gridy = 0;
        time.add(reset, gridConst);

        gridConst.gridx = 0;
        gridConst.gridy = 1;
        time.add(timeLabel, gridConst);

        time.setVisible(true);
    }

    public void actionPerformed(ActionEvent evt) {
        if (evt.getActionCommand().equalsIgnoreCase("Start")) {
            startStop.setActionCommand("Stop");
            startStop.setText("Stop");
            if (myControl.getCurMil() == -1)
                myControl.setCurMil(Calendar.getInstance().getTimeInMillis());
            else
                myControl.setCurMil(Calendar.getInstance().getTimeInMillis() - myControl.getDiff());
            myControl.count = true;

        } else if (evt.getActionCommand().equalsIgnoreCase("Stop")) {
            startStop.setActionCommand("Start");
            startStop.setText("Start");
            myControl.count = false;

        } else if (evt.getActionCommand().equalsIgnoreCase("Reset")) {
            startStop.setActionCommand("Start");
            startStop.setText("Start");
            myControl.count = false;
            myControl.setCurMil(-1);
            timeLabel.setText(prefix + "00:00:000" + suffix);
        }
    }

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

    }
}

class StopWatch implements Runnable {
    private long curMil;
    private long diff;
    private JLabel timeLabel;

    // *************** this is key ****************
    volatile boolean count; // !! *********************

    String prefix = "<html><body><h1>";
    String suffix = "</h1></body></html>";

    StopWatch(JLabel TimeLabel) {
        this.timeLabel = TimeLabel;
        this.count = false;
        this.curMil = -1;
    }

    public long getCurMil() {
        return curMil;
    }

    public void setCurMil(long curMil) {
        this.curMil = curMil;
    }

    public long getDiff() {
        return diff;
    }

    public void run() {
        while (true) {
            // System.out.println("Commenting this line will make stop watch useless");
            if (count) {
                diff = Calendar.getInstance().getTimeInMillis() - curMil;

                // make Swing changes **on** the event thread only
                SwingUtilities.invokeLater(() -> {
                    int mSec = (int) (diff % 1000);
                    int sec = (int) ((diff / 1000) % 60);
                    int min = (int) ((diff / (60 * 1000)) % 60);
                    String text = String.format("%s%02d:%02d:%03d%s", prefix, min, sec, mSec, suffix);
                    timeLabel.setText(text);
                });
                try {
                    Thread.sleep(1);  // ** actually 1 is OK **
                } catch (InterruptedException e) {
                    e.printStackTrace();
                }
            }
        }
    }
}

Upvotes: 3

Related Questions