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

Provide an abstraction for the system clock #29139

Closed
lukepuplett opened this issue Apr 1, 2019 · 5 comments
Closed

Provide an abstraction for the system clock #29139

lukepuplett opened this issue Apr 1, 2019 · 5 comments
Labels
api-needs-work API needs work before it is approved, it is NOT ready for implementation api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.DateTime
Milestone

Comments

@lukepuplett
Copy link

Should we have a standard abstraction for the wall-clock that returns the local DateTimeOffset?

The idea is to break the hard dependency on the system clock and be able to inject a different clock for system level testing of time and ageing impacts, especially useful in finance.

I've rarely seen anyone do this but often seen people that wish they had. I think its just a habit we haven't learned yet.

If we saw more types taking IWallClock or whatever we'd maybe think more about this. Sure its super easy to implement but like ILogFactory life's better when we're all using a uniform interface.

@Clockwork-Muse
Copy link
Contributor

Side note: Both Java (Clock - also has a timezone property) and NodaTime (IClock) have this, so the precedent is definitely there.

I feel like, instead of an interface or class, I want to have a delegate/func definition, but that's possibly just my fascination with functional languages talking. So something like:

public class SomeClass
{
    int SomeMethod(Func<DateTimeOffset> currentTime, int otherParameter);
    // To abstract getting the "system" timezone, if relevant.
    int SomeMethod(Func<(DateTimeOffset, TimeZoneInfo)> currentTime, int otherParameter);
}

// Used like:
var result = someClassInstance.SomeMethod(() => DateTimeOffset.UtcNow, 4);
var constant = someClassInstance.SomeMethod(() => new DateTimeOffset(2019, 4, 1, 15, 39, 0, TimeSpan.FromHours(0)), 4);

var result = someClassInstance.SomeMethod(() => (DateTimeOffset.UtcNow, TimeZoneInfo.Utc), 4);

...presumably, there'd be at least a "common" set of pre-defined clocks, for the system (probably provided by us?), as well as some other common use cases (fixed is trivial to create on your own if it's defined as a Func, but tick-every-n is slightly more complicated).

@vcsjones
Copy link
Member

vcsjones commented Apr 2, 2019

For what its worth this already exists in ASP.NET Core as ISystemClock. Personally I am not sold on this abstraction being in CoreFX since it's fairly easy for a developer to write themselves.

@danmoseley
Copy link
Member

Sure its super easy to implement but like ILogFactory life's better when we're all using a uniform interface

Is it so common for unrelated components to want to share such an interface, that itis burdensome for it to be shared in some other common library? The fact that something might be interesting to have and possibly share is not enough by itself to add to the base libraries. There is a cost to defining and shipping a type and we may make a bad decision about its shape. Once we ship an IClock in .NET Core itself we essentially claim the name, even for ourselves if we want to try again with a different design.

@lukepuplett
Copy link
Author

Valid points. I am not here to push my case, only to gather viewpoints.

A Func<DateTimeOffset> would work however, it requires a well-named parameter to communicate what the function should do, but maybe that's okay.

To avoid a function in a constructor or method signature, I think for many people it would mean writing a package that exports only a single public interface like IClock (to avoid taking a dependency on all of NodaTime e.g.)

But I'm open minded to people's thoughts, esp. contributors/maintainers.

@msftgits msftgits transferred this issue from dotnet/corefx Feb 1, 2020
@msftgits msftgits added this to the 5.0 milestone Feb 1, 2020
@maryamariyan maryamariyan added the untriaged New issue has not been triaged by the area owner label Feb 23, 2020
@joperezr joperezr added api-needs-work API needs work before it is approved, it is NOT ready for implementation and removed untriaged New issue has not been triaged by the area owner labels Jul 7, 2020
@joperezr joperezr modified the milestones: 5.0.0, Future Jul 7, 2020
@terrajobst
Copy link
Contributor

Let's close this in favor of #36617 (because it has more comments)

@ghost ghost locked as resolved and limited conversation to collaborators Dec 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-needs-work API needs work before it is approved, it is NOT ready for implementation api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.DateTime
Projects
None yet
Development

No branches or pull requests

9 participants