Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace FakeStopwatch with Clock.newStopwatch #126

Open
seaneagan opened this issue Apr 15, 2014 · 2 comments
Open

Replace FakeStopwatch with Clock.newStopwatch #126

seaneagan opened this issue Apr 15, 2014 · 2 comments

Comments

@seaneagan
Copy link
Contributor

Time-dependent units can receive Clocks as dependencies to mock the passage of time. Doing so exposes nothing about their implementation, only that they depend on the passage of time. #98 removed the need to expose the use of Timers. This issue is to remove the need to expose the use of Stopwatches. Currently quiver has FakeStopwatch for mocking stopwatches. Receiving a Stopwatch (FakeStopwatch) as a dependency, exposes even more of the implementation, specifically that you're using only a single stopwatch. A StopwatchFactory typedef would at least solve that, but it still would expose the use of Stopwatches.

If it were possible to obtain a Stopwatch from a Clock, the time-dependen unit could then just receive a Clock, and create the Stopwatch internally.

Ideally the dart:core Stopwatch impl would be backed by a Clock, see:

https://code.google.com/p/dart/issues/detail?id=18149

But until then, we will want to use the dart:core stopwatch impl in production code, and a clock-backed Stopwatch in test code. The way to handle this is polymorphism, by adding a stopwatch creation method to Clock:

Stopwatch newStopwatch();

createStopwatch could also work as a name.

It should be a method instead of a getter, since it returns a new mutable object on each invocation.

@trinarytree
Copy link
Contributor

Minor problem: FakeStopwatch currently is in the testing directory but Clock isn't, and most people probably don't want test code in production. Presumably, createStopwatch would create a FakeStopwatch. Part of the problem may be that FakeStopwatch has a prejudicial name. Like Clock, nothing is really "fake" about it, it's just driven by the now function it's given.

So maybe this would be a natural migration path:

  • Rename FakeStopwatch to something more neutral-sounding, like DelegatingStopwatch.
  • Move it closer to Clock so that it doesn't look like test-only code.
  • Add Stopwatch createStopwatch() to Clock.

@tolotrasamuel
Copy link

Is quiver not maintained anymore ? What is the outcome of this issue ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants