A simple Java class for implementing code stop watches

Question

(The next iteration: A simple Java class for implementing code stop watches - follow-up)

Here is my attempt at more convenient benchmarking (yes, this time I am aware of JVM warmup, etc.), and it looks like this:

package net.coderodde.stopwatch;
public final class StopWatch {
 private long earliestMillis = Long.MAX_VALUE;
 public StopWatch() {
 this(System.currentTimeMillis());
 } 
 public StopWatch(long earliestMillis) {
 this.earliestMillis = earliestMillis;
 }
 public void push(long earliestMillis) {
 if (earliestMillis < this.earliestMillis) {
 throw new IllegalArgumentException("Cannot go back in time.");
 }
 this.earliestMillis = earliestMillis;
 }
 public long pop() {
 return System.currentTimeMillis() - earliestMillis;
 }
 public long popAndPush() {
 long ret = pop();
 this.earliestMillis = System.currentTimeMillis();
 return ret;
 }
 @Override
 public String toString() {
 StringBuilder stringBuilder = new StringBuilder();
 stringBuilder.append("(")
 .append(System.currentTimeMillis() - earliestMillis)
 .append(" starting from ")
 .append(earliestMillis)
 .append(" ms -> ")
 .append(System.currentTimeMillis())
 .append(")");
 return stringBuilder.toString();
 }
 public static void main(String[] args) throws InterruptedException {
 StopWatch sw = new StopWatch();
 Thread.sleep(3_000L);
 System.out.println(sw.pop());
 System.out.println(sw);
 Thread.sleep(2_000L);
 System.out.println(sw.pop());
 sw.popAndPush();
 Thread.sleep(1_230L);
 System.out.println(sw);
 }
}

Is there more reasonable/extensive approach to this?

• danielspaniol: pop and push are really strange names for resetting and reading a stopwatch as those names imply a stack-like structure.
• AJNeufeld: Additional gripe: push() and popAndPush() are asymmetrical. The former takes an argument to be pushed, but latter doesn't take any argument.

Answer 1

public final class StopWatch {
 private long earliestMillis = Long.MAX_VALUE;

There is no reason to initialize this field at this point. Both constructors will overwrite it immediately.

 public StopWatch() {
 this(System.currentTimeMillis());
 } 
 public StopWatch(long earliestMillis) {
 this.earliestMillis = earliestMillis;
 }

There should be no reason for the user to provide their own initial start time. The only reason could be to use an entirely different clock for testing this code. For that case, you should use a MillisProvider instead of calling System.currentTimeMillis directly. As of now, you have a wild mixture of calling currentTimeMillis and accepting caller-provided timestamps, which can easily lead to confusion.

 public void push(long earliestMillis) {

Again, I don't see any reason not to hard-code System.currentTimeMillis here.

 if (earliestMillis < this.earliestMillis) {
 throw new IllegalArgumentException("Cannot go back in time.");
 }

When you measure code for performance, you usually don't expect additional exceptions to be thrown. Changing the system clock backwards should not happen anyway, but still this code should deal a little softer with this situation by just ignoring it.

 this.earliestMillis = earliestMillis;
 }
 public long pop() {
 return System.currentTimeMillis() - earliestMillis;
 }
 public long popAndPush() {
 long ret = pop();
 this.earliestMillis = System.currentTimeMillis();
 return ret;
 }
 @Override
 public String toString() {
 StringBuilder stringBuilder = new StringBuilder();
 stringBuilder.append("(")
 .append(System.currentTimeMillis() - earliestMillis)
 .append(" starting from ")
 .append(earliestMillis)
 .append(" ms -> ")
 .append(System.currentTimeMillis())
 .append(")");
 return stringBuilder.toString();
 }

Using a StringBuilder here is unnecessary. It is only needed if you have some parts that are added conditionally to the string. Just use plain string concatenation, which is far easier to read. Alternatively, since the toString method is probably not called in the performance-critial part, it's probably ok to use String.format here, to separate the general format of the message from the parameters.

Oh, I just saw that toString calls currentTimeMillis. That's wrong. It's even wronger to call it twice in a row (and expecting both calls to return the same timestamp). Measuring the time should be as fast and light-weight as possible. Building a nice presentation out of the raw measurement values should be clearly separated from the measurement.

To increase the precision of the measurement, you should determine how long it takes to call currentTimeMillis and subtract that from the reported durations.

 public static void main(String[] args) throws InterruptedException {
 StopWatch sw = new StopWatch();
 Thread.sleep(3_000L);
 System.out.println(sw.pop());
 System.out.println(sw);
 Thread.sleep(2_000L);
 System.out.println(sw.pop());
 sw.popAndPush();
 Thread.sleep(1_230L);
 System.out.println(sw);
 }
}

As danielspaniol already said in a comment, having a stopwatch with buttons labelled "push" and "pop" feels strange. Looking at Wikipedia, the second image, your methods should rather be called start, stop, reset, lap, split, recall, pause.