Make `Stopwatch` logic simpler.

Make `Stopwatch` logic simpler.

The `Stopwatch` class had more states than necessary because it started with
the `_start` and `_stop` fields being `null`, a state that couldn't happen later,
and it had to handle those cases specially.
Now just starts in a state equivalent to a stopped, reset timer.
Also use ?? and ??= because they are there,and don't call the overridable `isRunning` from other methods.

R=floitsch@google.com

Committed: https://github.com/dart-lang/sdk/commit/c3bb3ddb4184d8bd707a6752741c549c17f9c55e

[Lasse Reichstein Nielsen, 2016-11-08 14:23:49]

lrn@google.com changed reviewers:
+ floitsch@google.com

[Lasse Reichstein Nielsen, 2016-11-08 14:23:49]

I looked at the class for another reason and thought it looked too complicated.

[Lasse Reichstein Nielsen, 2016-11-09 06:23:43]

Description was changed from

==========
Make `Stopwatch` logic simpler.

The `Stopwatch` class had more states than necessary because it started with
the `_start` and `_stop` fields being `null`, a state that couldn't happen
later,
and it had to handle those cases specially.
Now just starts in a state equivalent to a stopped reset timer.
Also use ?? and ??= because they are there and don't call the overridable
`isRunning`
from other methods.
==========

to

==========
Make `Stopwatch` logic simpler.

The `Stopwatch` class had more states than necessary because it started with
the `_start` and `_stop` fields being `null`, a state that couldn't happen
later,
and it had to handle those cases specially.
Now just starts in a state equivalent to a stopped, reset timer.
Also use ?? and ??= because they are there and don't call the overridable
`isRunning`
from other methods.
==========

[Lasse Reichstein Nielsen, 2016-11-09 11:35:10]

Would it make sense to add a method that stops *and* resets at  the same time?
(It would save one wasted call to _now()).

[Lasse Reichstein Nielsen, 2016-11-09 12:36:43]

Description was changed from

==========
Make `Stopwatch` logic simpler.

The `Stopwatch` class had more states than necessary because it started with
the `_start` and `_stop` fields being `null`, a state that couldn't happen
later,
and it had to handle those cases specially.
Now just starts in a state equivalent to a stopped, reset timer.
Also use ?? and ??= because they are there and don't call the overridable
`isRunning`
from other methods.
==========

to

==========
Make `Stopwatch` logic simpler.

The `Stopwatch` class had more states than necessary because it started with
the `_start` and `_stop` fields being `null`, a state that couldn't happen
later,
and it had to handle those cases specially.
Now just starts in a state equivalent to a stopped, reset timer.
Also use ?? and ??= because they are there,and don't call the overridable
`isRunning` from other methods.
==========

[Lasse Reichstein Nielsen, 2016-11-09 13:29:08]

https://codereview.chromium.org/2485063003/diff/20001/sdk/lib/core/stopwatch....
File sdk/lib/core/stopwatch.dart (right):

https://codereview.chromium.org/2485063003/diff/20001/sdk/lib/core/stopwatch....
sdk/lib/core/stopwatch.dart:14: static int _frequency;
Moved field to here, getter below constructor.

[floitsch, 2016-11-10 16:31:57]

LGTM.

https://codereview.chromium.org/2485063003/diff/20001/sdk/lib/core/stopwatch....
File sdk/lib/core/stopwatch.dart (right):

https://codereview.chromium.org/2485063003/diff/20001/sdk/lib/core/stopwatch....
sdk/lib/core/stopwatch.dart:14: static int _frequency;
On 2016/11/09 13:29:08, Lasse Reichstein Nielsen wrote:
> Moved field to here, getter below constructor.

Acknowledged.

https://codereview.chromium.org/2485063003/diff/20001/sdk/lib/core/stopwatch....
sdk/lib/core/stopwatch.dart:18: // If _stop is null, the stopwatched is running.
stopwatch

https://codereview.chromium.org/2485063003/diff/20001/sdk/lib/core/stopwatch....
sdk/lib/core/stopwatch.dart:27: * ```dart
No need for "dart".
That should be the default in dartdocs.

https://codereview.chromium.org/2485063003/diff/20001/sdk/lib/core/stopwatch....
sdk/lib/core/stopwatch.dart:28: * Stopwatch stopwatch = new
Stopwatch()..start();
Maybe switch to 'var'.

https://codereview.chromium.org/2485063003/diff/20001/sdk/lib/core/stopwatch....
sdk/lib/core/stopwatch.dart:51: // (Re)start this stopwatch. Prepend the elapsed
time to the current
Maybe?

(Re)start this stopwatch. Move the start-point forward by the gap since the
watch was stopped.

In the end it doesn't really matter, though.

https://codereview.chromium.org/2485063003/diff/20001/sdk/lib/core/stopwatch....
sdk/lib/core/stopwatch.dart:66: _stop ??= _now();
I found the `isRunning` more telling, but the file is small enough to see what
happens.

https://codereview.chromium.org/2485063003/diff/20001/sdk/lib/core/stopwatch....
sdk/lib/core/stopwatch.dart:116: * Returns whether the [StopWatch] is currently
running.
Remove `Returns`.

[Lasse Reichstein Nielsen, 2016-11-11 09:00:55]

https://codereview.chromium.org/2485063003/diff/20001/sdk/lib/core/stopwatch....
File sdk/lib/core/stopwatch.dart (right):

https://codereview.chromium.org/2485063003/diff/20001/sdk/lib/core/stopwatch....
sdk/lib/core/stopwatch.dart:27: * ```dart
On 2016/11/10 16:31:57, floitsch wrote:
> No need for "dart".
> That should be the default in dartdocs.

Done.

https://codereview.chromium.org/2485063003/diff/20001/sdk/lib/core/stopwatch....
sdk/lib/core/stopwatch.dart:28: * Stopwatch stopwatch = new
Stopwatch()..start();
On 2016/11/10 16:31:57, floitsch wrote:
> Maybe switch to 'var'.

Done.

https://codereview.chromium.org/2485063003/diff/20001/sdk/lib/core/stopwatch....
sdk/lib/core/stopwatch.dart:51: // (Re)start this stopwatch. Prepend the elapsed
time to the current
Reworeded to:
... Don't count the time while the stopwatch has been stopped.

https://codereview.chromium.org/2485063003/diff/20001/sdk/lib/core/stopwatch....
sdk/lib/core/stopwatch.dart:66: _stop ??= _now();
It is more readable, but it's also calling an overridable member which is bad
style.
We could add a private _isRunning, but I don't think it's worth it.

The real problem is probably that you can extend Stopwatch, but that's a 2.0
change if we want to prohibit that.

https://codereview.chromium.org/2485063003/diff/20001/sdk/lib/core/stopwatch....
sdk/lib/core/stopwatch.dart:116: * Returns whether the [StopWatch] is currently
running.
Removed everywhere.

[Lasse Reichstein Nielsen, 2016-11-11 09:01:34]

Description was changed from

==========
Make `Stopwatch` logic simpler.

The `Stopwatch` class had more states than necessary because it started with
the `_start` and `_stop` fields being `null`, a state that couldn't happen
later,
and it had to handle those cases specially.
Now just starts in a state equivalent to a stopped, reset timer.
Also use ?? and ??= because they are there,and don't call the overridable
`isRunning` from other methods.
==========

to

==========
Make `Stopwatch` logic simpler.

The `Stopwatch` class had more states than necessary because it started with
the `_start` and `_stop` fields being `null`, a state that couldn't happen
later,
and it had to handle those cases specially.
Now just starts in a state equivalent to a stopped, reset timer.
Also use ?? and ??= because they are there,and don't call the overridable
`isRunning` from other methods.

R=floitsch@google.com

Committed:
https://github.com/dart-lang/sdk/commit/c3bb3ddb4184d8bd707a6752741c549c17f9c55e
==========

[Lasse Reichstein Nielsen, 2016-11-11 09:01:42]

Committed patchset #2 (id:20001) manually as
c3bb3ddb4184d8bd707a6752741c549c17f9c55e (presubmit successful).