Inefficient Stopwatch - revised

Question

This is my revised code of Stopwatch:

package com.zm0617;

import java.awt.*;

import javax.swing.*;

import javax.swing.border.EmptyBorder;

import java.awt.event.*;

public class MultiThreadedStopwatchController implements StopwatchController,
        StopwatchState {

    private volatile static boolean isStarted;

    public MultiThreadedStopwatchController() {
        isStarted = false;
    }

    public void stop() {
        isStarted = false;
    }

    public void start() {
        isStarted = true;
    }

    public void toggle() {
        if (isStarted) {
            isStarted = false;
        } else {
            isStarted = true;
        }
    }

    public boolean isStarted() {
        return isStarted;
    }
}

interface StopwatchState {
    boolean isStarted();
}

interface StopwatchController {
    void stop();

    void start();

    void toggle();

    boolean isStarted();
}

class Clock implements Runnable {

    private StopwatchState state;

    static int hr = 0;
    static int min = 0;
    static int sec = 0;
    static double milisec = 0;
    static int rotation = 0;
    static long time1 = 0;
    static long time2 = 0;

    public Clock(StopwatchState state) {
        this.state = state;
    }

    @Override
    public void run() {
        while (true) {
            time1 = System.currentTimeMillis();
            while (state.isStarted()) {
                if (!state.isStarted()) {
                    break;
                }
                if (rotation == 0) {
                    try {
                        Thread.sleep(5);
                    } catch (InterruptedException e) {
                        Thread.interrupted(); // interrupt state to false
                        break;
                    }
                    time2 = System.currentTimeMillis();
                    plus((int) time2 - (int) time1);
                    rotation++;
                } else if (rotation == 1) {
                    try {
                        Thread.sleep(5);
                    } catch (InterruptedException e) {
                        Thread.interrupted(); // interrupt state to false
                        break;
                    }
                    time1 = System.currentTimeMillis();
                    plus((int) time1 - (int) time2);
                    rotation--;
                }
            }
            if (Stopwatch.resets()) {
                hr = 0;
                min = 0;
                sec = 0;
                milisec = 0;

            }
        }
    }

    public static void plus(int i) {
        milisec += i;
        if (milisec >= 1000) {
            milisec -= 1000;
            sec++;
        }
        if (sec >= 60) {
            sec -= 60;
            min++;
        }
        if (min >= 60) {
            min -= 60;
            hr++;
        }
    }

    public static String getHr() {
        return toStringHMS(hr);
    }

    public static String getMin() {
        return toStringHMS(min);
    }

    public static String getSec() {
        return toStringHMS(sec);
    }

    public static String getMilisec() {
        return toStringM(milisec);
    }

    public static String toStringHMS(Integer i) {
        String s = i.toString();
        if (s.length() == 1) {
            s = "0" + s;
        }
        return s;
    }

    public static String toStringM(Double d) {
        Integer i = (int) Math.round(d);
        String s = i.toString();
        if (s.length() == 1) {
            s = "00" + s;
        } else if (s.length() == 2) {
            s = "0" + s;
        }
        return s;
    }
}

class Stopwatch extends JFrame {

    /**
     * 
     */
    private static final long serialVersionUID = 1L;

    private static StopwatchController controller;

    final static MultiThreadedStopwatchController stopwatchController = 
                                             new MultiThreadedStopwatchController();
    static Clock clock = new Clock(stopwatchController);
    static Thread t = new Thread(clock);

    private JPanel contentPane;
    private static JTextField hr;
    private static JTextField min;
    private static JTextField sec;
    private static JLabel milisec;
    private static JButton start;
    static boolean breset = false;
    static boolean frameDone = false;

    /**
     * Launch the application.
     */
    public static void main(String[] args) {
        EventQueue.invokeLater(new Runnable() {
            public void run() {
                try {
                    Stopwatch frame = new Stopwatch(stopwatchController);
                    frame.setVisible(true);
                } catch (Exception e) {
                    e.printStackTrace();
                }
            }
        });
        t.setDaemon(true);
        t.start();
        while (true) {
            if (frameDone) {
                if (controller.isStarted()) {
                    start.setText("Stop");
                } else {
                    start.setText("Start");
                }
                hr.setText(Clock.getHr());
                min.setText(Clock.getMin());
                sec.setText(Clock.getSec());
                milisec.setText(Clock.getMilisec());
            }
            System.out.print("");
        }
    }

    /**
     * Create the frame.
     */
    public Stopwatch(final StopwatchController controller) {

        Stopwatch.controller = controller;

        setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        setBounds(100, 100, 200, 110);
        contentPane = new JPanel();
        contentPane.setBorder(new EmptyBorder(5, 5, 5, 5));
        setContentPane(contentPane);
        contentPane.setLayout(new FlowLayout(FlowLayout.CENTER, 5, 5));

        hr = new JTextField();
        hr.setEditable(false);
        hr.setText("00");
        contentPane.add(hr);
        hr.setColumns(3);

        JLabel colona = new JLabel(":");
        contentPane.add(colona);

        min = new JTextField();
        min.setEditable(false);
        min.setText("00");
        contentPane.add(min);
        min.setColumns(3);

        JLabel colonb = new JLabel(":");
        contentPane.add(colonb);

        sec = new JTextField();
        sec.setEditable(false);
        sec.setText("00");
        contentPane.add(sec);
        sec.setColumns(3);

        milisec = new JLabel("000");
        milisec.setFont(new Font("Tahoma", Font.PLAIN, 8));
        contentPane.add(milisec);

        JButton reset = new JButton("Reset");
        reset.addMouseListener(new MouseAdapter() {
            @Override
            public void mouseReleased(MouseEvent arg0) {
                if (!controller.isStarted()) {
                    breset = true;
                }
            }
        });
        contentPane.add(reset);

        start = new JButton("Start");
        start.addMouseListener(new MouseAdapter() {
            @Override
            public void mouseReleased(MouseEvent e) {
                controller.toggle();
            }
        });
        contentPane.add(start);
        frameDone = true;
    }

    public static boolean resets() {
        if (breset) {
            breset = false;
            t.interrupt();
            return true;
        } else {
            return false;
        }
    }
}

Answer 1

Going from your previous question on the topic, I'm tempted to point you back to the accepted answer but, as you pointed out in a comment, Java is not (currently) your strong point, so the answer may have left you dazed.

So let's take it from the top.

---

Static vs. Instance

From the official tutorial on static:

Sometimes, you want to have variables that are common to all objects. This is accomplished with the static modifier. Fields that have the static modifier in their declaration are called static fields or class variables. They are associated with the class, rather than with any object. Every instance of the class shares a class variable, which is in one fixed location in memory. Any object can change the value of a class variable, but class variables can also be manipulated without creating an instance of the class.

Statics live in the scope of an entire application⁽¹⁾. Anything that belongs or relates to a specific instance must not be static.

A truck has wheels that are independent of other trucks with wheels. A moment in time has seconds, minutes, hours that are independent of any other moment in time. A stopwatch measures elapsed time as per when start and stop are called on that stopwatch, so the static has to go.

(1) _{The classloader level, more accurately, but let's not wade into that morass.}

---

Thread safety

Swing GUI objects, such as JLabel,⁽²⁾ must be updated on the Event Dispatch Thread (EDT):

Swing event handling code runs on a special thread known as the event dispatch thread. Most code that invokes Swing methods also runs on this thread. This is necessary because most Swing object methods are not "thread safe": invoking them from multiple threads risks thread interference or memory consistency errors. [...] Programs that ignore this rule may function correctly most of the time, but are subject to unpredictable errors that are difficult to reproduce.

Better to handle any manipulation of graphical components through the EDT.

(2) _{Earlier versions of Java held JTextField.setText(String) to be thread-safe, but this guarantee was dropped in 7 and later.}

---

Overall Design

Think about what you need. Drill it down to the barest essence. At the very minimum, you will need two concepts: one to represent your stopwatch, and one to represent your GUI. These are the two major entities in your design. Everything else is basically frills.

A more subtle consideration, but very defining for your design, is who is responsible for relaying the passage of time. Does the stopwatch send out signals when its state changes? Do interested parties query once every while? Either approach has its pros and cons, so it's a matter of insight and preference.

Let's stay with your original choice: the stopwatch is a passive construct and does not send out signals; this means that interested parties must poll for changes. Helpful here will be javax.swing.Timer, a timer that's designed to be Swing-safe.

Regardless of what approach you take, the stopwatch design will be pivotal to both entities:

// Alternatively, instead of start()/stop(), consider setRunning(boolean)
interface IStopwatch {
  /** Total time the stopwatch has been running since the last call to reset(). */
  long getElapsedTime();
  /** Starts adding to elapsed time. */
  void start();
  /** Stops adding to elapsed time. */
  void stop();
  /** Resets elapsed time to zero. */
  void reset();
  /** Whether the stopwatch is currently running / accumulating time. */
  boolean isRunning();
}

Sample implementation

Here's a quick-and-dirty sample implementation that uses polling to get the stopwatch's state. Note that the example implementation of the stopwatch is not inherently thread-safe, but is created and manipulated solely from the EDT.

public class StopwatchFrame extends JFrame {
  // Do not confuse with java.util.Timer !
  javax.swing.Timer timer;

  IStopwatch stopwatch;

  JLabel time;
  JToggleButton stopStart;
  JButton reset;

  public StopwatchFrame(IStopwatch stopwatch) {
    this.stopwatch = stopwatch;

    getContentPane().setLayout(new FlowLayout());

    time = new JLabel();
    add(time);

    stopStart = new JToggleButton("Running?");
    stopStart.addActionListener(new ActionListener() {
      public void actionPerformed(ActionEvent evt) {
        if ( stopStart.isSelected() ) {
          stopwatch.start();
        } else {
          stopwatch.stop();
        }
      }
    });
    add(stopStart);

    reset = new JButton("Reset");
    reset.addActionListener(new ActionListener() {
      public void actionPerformed(ActionEvent evt) {
        stopwatch.reset();
      }
    });
    add(reset);

    setFields();

    timer = new javax.swing.Timer(15, new ActionListener() {
      public void actionPerformed(ActionEvent evt) {
        setFields();
      }
    });
    timer.start();
  }

  void setFields() {
    long elapsedTime = stopwatch.getElapsedTime();

    final long millis = elapsedTime % 1000;
    elapsedTime /= 1000;
    final long seconds = elapsedTime % 60;
    elapsedTime /= 60;
    final long minutes = elapsedTime % 60;
    elapsedTime /= 60;
    final long hours = elapsedTime;

    setFields(hours, minutes, seconds, millis, stopwatch.isRunning());
  }

  void setFields(long hours, long minutes, long seconds, long millis, boolean running) {
    time.setText(String.format("<html>%02d:%02d:%02d<font size=2>:%03d", hours, minutes, seconds, millis));
    stopStart.setSelected(running);
  }

  public static void main(String[] args) {
    SwingUtilities.invokeLater(new Runnable() {
      public void run() {
        final IStopwatch stopwatch = new Stopwatch();

        StopwatchFrame frame = new StopwatchFrame(stopwatch);
        frame.pack();
        frame.setDefaultCloseOperation(WindowConstants.EXIT_ON_CLOSE);
        frame.setVisible(true);

        frame = new StopwatchFrame(stopwatch);
        frame.pack();
        frame.setDefaultCloseOperation(WindowConstants.EXIT_ON_CLOSE);
        frame.setVisible(true);
      }
    });
  }
}

class Stopwatch implements IStopwatch {
  long accumulatedMillis;
  long startedAt;
  boolean running;

  public void start() {
    if ( running ) {
      return; // ignore
    }

    running = true;
    startedAt = System.currentTimeMillis();
  }

  public void stop() {
    if ( !running ) {
      return;
    }

    running = false;
    final long now = System.currentTimeMillis();
    accumulatedMillis += now - startedAt;
  }

  public long getElapsedTime() {
    if ( running ) {
      return accumulatedMillis + System.currentTimeMillis() - startedAt;
    }

    return accumulatedMillis;
  }

  public void reset() {
    startedAt = System.currentTimeMillis();
    accumulatedMillis = 0;
  }

  public boolean isRunning() {
    return running;
  }
}