Clock has too many methods

[seaneagan]

Clock has too many methods

[yjbanov]

What we probably need is for Clock to be callable, so you could do:

clock().subtract(duration);

With this in place we could easily get rid of agoBy and fromNowBy.

There is some level of intentional redundancy in the API, e.g.:

clock.ago(days: 1);
clock.daysAgo(1);

*Ago and *FromNow are not strictly necessary. They are there for convenience only. For example, it is less code to pass weeksAgo function to other functions:

// Same day of week for the next 7 weeks
new List.generate(7, clock.weeksFromNow);

[seaneagan]

+1 to making Clock callable.

I think in most scenarios you are going to have to access the clock a single time, and pass the result around, otherwise the dates will all be off by a few milliseconds or so. I think that example would likely have to be:

var now = clock();
new List.generate(7, (i) => now.add(aWeek * i));

[yjbanov]

Is the existence of these extra methods causing problems?

[seaneagan]

Not at all. The reason I brought it up is I would like to see Clock in dart:core someday to support:

http://dartbug.com/16762

and that Clock would likely have very few methods. If that ever happened, then quiver's Clock would likely be deprecated, and not having the extra methods would make that transition a bit easier.

[seaneagan]

Could use top-level functions to allow defaulting to the system clock:

ago(Duration duration, [Clock clock = const Clock.system()]) => 
    clock.now().subtract(duration);
fromNow(Duration duration, [Clock clock = const Clock.system()]) => 
    clock.now().add(duration);

Then in scripts or whatever that don't need to have their time mocked, can just do:

fromNow(anHour)
ago(aDay)

Also regarding Clock potentially moving to dart:core in the future, see:

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

[yjbanov]

I agree. I'm ok with keeping this issue open, but I don't think we should be making any changes until something actually happens in the SDK.

[cbracken]

Closing as stale. This class is so heavily used internally that changing these APIs at this point that the benefit outweighs the cost. As mentioned elsewhere, I've always been a fan of offsetBy(Duration) or at least offsetBy({years, weeks, days, ...}) but I'm not sure we'll find many people keen to sign up for the thousands of patches we'd need to land to migrate :-)