StopWatch improvements

[DartBot]

This issue was originally filed by @seaneagan

---

interface StopWatch {

  const Stopwatch(); // change to "const" constructor
  // remove Stopwatch.start(), since issue #552 provides this convenience in a much more orthogonal way

  // use getters instead of methods for elapsed times
  int get ticks(); // more descriptive name
  int get hertz(); // replaces "frequency" with self-documenting name
  int get milliseconds(); // avoids abbreviations consistent with Date, Duration
  int get microseconds(); // ditto

  void start();
  void stop();
  void reset();

  // "lap" (or "split") functionality for timing loop iterations
  void lap(); // starts a new lap
  List<int> get laps();
  List<int> get millisecondLaps();
  List<int> get microsecondLaps();
}

[iposva-google]

Thanks for your suggestions. I'll take a look at them in more detail, still here are some first reactions:

const Stopwatch(); // change to "const" constructor
This is impossible as a const object could not be stopped, restarted or reset. Actually a const object is a compile time constant and thus it being a Stopwatch would be a bit pointless.

use getters instead of methods for elapsed time
I can definitely sympathize with using getters instead of methods. As far as the naming of the getters goes, to me the frequency is self-documenting: frequencies are measured in hertz. The others are probably fine to be renamed, I'll check the naming of similar things in the core libraries.

"lap" (or "split") functionality for timing loop iterations
The biggest problem with this adding a lap time feature is the fact that the Stopwatch ends up not being a very cheap interface any longer, for every lap you might need to do allocation to grow the backing array and suddenly the mere fact of measuring can disturb measurements.

---

Set owner to @iposva-google.
Added Area-Library, Accepted labels.

[DartBot]

This comment was originally written by @seaneagan

---

A couple things to add:

Should it just rely on the new Clock.frequency instead of having its own ?

Regarding lap/split functionality, the StopWatch could be stopped during array allocation. Could be implemented as something like:

class StopWatchImpl implements StopWatch {

  //...

  lap() {
    // record lap time / stop StopWatch
    int lapTime = _currentLap();
    // add lapTime to backing array
    _laps.add(lapTime);
    // cache the total elapsed time for all laps
    _total += lapTime;
    // start StopWatch for next lap
    _lapStart = Clock.now; // restart timing
  }
  int get ticks() => _total + _currentLap()
  List get laps() {
    if(_laps == null) _laps = new _LapList(this);
    return _laps;
  }

  _LapList _laps;
  int _lapStart;
  int _total;
  _currentLap() => Clock.now - _lapStart;
}

class _LapList implements List<int> {

  _LapList(this._stopWatch) => _internal = new List();

  List<int> _internal;
  StopWatch _stopWatch

  int last() => _stopWatch._currentLap();
  int get length() _internal.length + 1;
  //... etc.

}

[DartBot]

This comment was originally written by @seaneagan

---

Also, a Duration getter:

Duration get elapsed();

could replace the millisecond and microsecond getters (and add a bunch more for longer running StopWatches):

stopWatch.elapsed.inMicroseconds; // would need to be added to Duration
stopWatch.elapsed.inMilliseconds;
stopWatch.elapsed.inSeconds;
stopWatch.elapsed.inMinutes;
// etc.

[DartBot]

This comment was originally written by @seaneagan

---

Could even add the clock ticks getter to Duration:

// tick frequency is same as [Clock.frequency]
int get inTicks();

Then to get the elapsed ticks, just do:

stopWatch.elapsed.inTicks

[DartBot]

This comment was originally written by @seaneagan

---

So in summary, it might look something like:

interface Stopwatch {
  void start();
  void stop();
  void reset();
  Duration get elapsed();

  void split();
  List<Duration> get splits();
}

[DartBot]

This comment was originally written by @mdakin

---

I propose adding a deltaInMs() method which would return the time between now and last call of delta (or start)

Rationale:

For benchmarking I am interested in finding deltas between measurements. The pattern is usually something like this:

Stopwatch w = new Stopwatch();
w.start();
<Operation 1>
print("Operation 1: ${w.elapsedInMs()} ms.");
w.reset();
<Operation 2>
print("Contains: ${w.elapsedInMs()} ms.");

This approach has a problem that I lose elapsed time from beginning. and I have to call reset() before making incremental measurements.

so Instead, we could have:

Stopwatch w = new Stopwatch();
w.start();
<Operation 1>
print("Operation 1: ${w.deltaInMs()} ms.");
<Operation 2>
print("operation 2: ${w.deltaInMs()} ms.");
...
print("Total elapsed time: ${w.deltaInMs()} ms.");

The only additional burden is an extra integer in the class.

[DartBot]

This comment was originally written by @mdakin

---

Sorry, there is an error in my example (last line should have used elapsedInMS():

Current Stopwatch implementation:

Stopwatch w = new Stopwatch();
w.start();
<Operation 1>
print("Operation 1: ${w.elapsedInMs()} ms.");
w.reset();
<Operation 2>
print("operation 2: ${w.elapsedInMs()} ms.");

with deltaInMs():

Stopwatch w = new Stopwatch();
w.start();
<Operation 1>
print("Operation 1: ${w.deltaInMs()} ms.");
<Operation 2>
print("operation 2: ${w.deltaInMs()} ms.");
...
print("Total elapsed time: ${w.elapsedInMs()} ms.");

[DartBot]

This comment was originally written by @seaneagan

---

@7: Your "deltaInMs" is basically the same as my "split" above, except you are returning the value rather than storing it in a List. Method names should be verbs, so I think "split" is better than "deltaInMs":

int split();

I also would prefer using a Duration:

Duration split();

and I still would see value in storing the splits:

List<Duration> get splits();

but those are separate issues.

[DartBot]

This comment was originally written by @mdakin

---

I think keeping the class as simple as possible is a good idea, I asked for deltaInMs() because it is convenient and name matches elapsedInMs().

split() doesn't sound good for this use case: What are we splitting?
print("operation 1 took: ${w.split()} ms.");

[DartBot]

This comment was originally written by @seaneagan

---

Physical stopwatches can record split (aka lap) times, see http://en.wikipedia.org/wiki/Stopwatch. A more explicit name might be better, such as "recordSplit" or "getSplit". I don't think abbreviations are generally a good idea, so I think any "InMs" suffix would really need to be "InMilliseconds" which is too long, and anyway unnecessary, especially if using Durations instead of ints.

[sethladd]

cc @lrhn.

[lrhn]

We have no current plan to add more improvements to StopWatch.
The getters are now getters, but laps/splits will have to be handled externally.
It should be fairly simple to wrap a StopWatch with something handling splits, I'd prefer to keep the base class simple.

---

Added NotPlanned label.