As someone who works with both hardware and software, I'm going to agree with their decisions. "Clock" has different meanings in hardware - does it provide a tick or a time? ITimeProvider makes that abundantly clear. AS for the second, Properties inherently signal that no work is done when retrieving the value, but there are definitely situations where that's not true. What if I'm using an NtpTimeProvider or some other provider that does some real work, that could take real time or resources to retrieve? A method makes that way, way more clear IMO
I especially love the comment here about engaging with the team to voice opinion. Be vocal about what you think! One minor change I would add though - make sure the discourse is respectful and calm, otherwise nobody will listen.
I think the reason they made it an abstract class is that that's what their internal guidelines say to prefer -- because newer versions of the framework can add new properties to a class, but not to an interface... Obviously they wrote that back before we had default interface methods! I wonder if they've re-evaluated their guidance since. Still, both are easy to mock, so it mostly comes down to style.
I suppose that it is called TimeProvider because is like a provider for the "time" dimension in the "spacetime". Why? Because the TimestampFrequency parameter, the CreateTimer method and because of their implementation of FakeTimeProvider who has also the Advance() method
Clock is bad name for me, as someone mentioned in the comments it can interface IDateTimeProvider (reason we working not only witch time but with date also) with two methods that will have prefix in name "Get" and they will be corresponding accordingly to the fields in DateTime instance.
Probably better to assume that DotNet programmers have a good reason for their naming conventions. In the case of ITimeProvider, the name is more fitting for a broader range of uses.
Good changes, but why bother developers implementing a abstract class for no particular reason. Therefore it should be an interface with two methods: GetUtcNow() and GetLocalNow() and thats it.
I think properties are clearer, and should be used instead of a GetUtcNow() method for concistency with other types, Also an Interface with a Default implementation so I can use a mocking framework and restrict some method / properties being used, e.g. Now that returns the current time on the server should not be called. Maybe add a method where you can pass in your own offset and/or Timezone so you know what Timezone will be returned. I personally don't understand why someone would use anything but UtcNow in a web application, except if the time presented is based on user location. Maybe also address the fact that we have to check what OS ve're on before retrieving a specific Timezone.
It's a time provider. That means, the time is not an intrinsic property of the object, it has to go somewhere else to look for it, same as an API client or a repository. On the contrary, a DateTime is an object that already contains the date and time information in itself, so those are properties. It's a matter of semantics in the end, a property is also a method behind the scenes, but it does make sense. A method clearly depicts that action is being done.
But if it had to look to an api or other external providers the return type should be Task. I rather go with a property instead of a parameterless sync method
I don't see how the call being synchronous or asynchronous makes any difference here. Are you saying that all parameterless synchronous methods should become properties?
Maybe I'm just slow on the uptake, but I did not understand the framing of this video at all. "The old implementation caused my unit tests to fail". Why? Was there anything in here that explained why they failed? If so, I missed it. "The new implementation fixed my unit tests". Again, why? I heard a lot of discussion about implementation styles and preferences, but what was the actual problem, and why did the new implementation solve it?
Doesn't always work. By the time you pass the time down, there is a delta between it being passed down and it being used, so it's going to be inaccurate
@@ReneWiersmaMusic You know as soon as people know of the TimeProvider, they will inject it everywhere, even if only the current DateTime is needed. So many things could just be a simple func. No dependencies, no interfaces, only exactly what is required. Been thinking about this lately, seeing some class with lots of injected interfaces where 1 method is called on each of them. It feels like this kind of code can be hard to understand because you don't know which calls are made on those interfaces. I wonder if it would be easier if we supplied only the dependencies that are required, but as Funcs instead of interfaces.
What do you mean by "source compatibility"? My understanding is that a property in the background operates exactly how a method's "GET" function does when returning a non-static value.
1:34 This is probably an unpopular opinion but I think Microsoft's love for internal types is excessive and detrimental to developers using the open source code, and in general I think the internal keyword is a bad practice that treats developers like babies and in all instances it should be changed to moving the type into a namespace (or otherwise separated) that identifies the type as internal but does not lock it off from legitimate tinkering.
Internal types would pollute intellisense to a point where it would be as bad as javascript, causing naming conflicts everywhere. We still have reflection for the 1:1000 chance your project needs some .NET internals exposed, also when you do that YOU are responsible if it breaks in next update and that is correct.
@@the-niker that's the developers are babies argument. I guess maybe they are. Yes you can use reflection. What's the difference in developer responsibility when using reflection and using a type clearly marked as internal (but not with the internal keyword specifically)? Reflection just makes it harder to know when things have been broken. I guess I'd settle for some syntactic sugar that allowed easy internal type usage with some compiler warning.
@@orterves Without reflection your production server will one day break after a random low-priority windows update and the person that inherits the project won't have a clue why. Proper safe reflection can isolate the call and keep the system alive just degraded and report the anomaly to logs with probable cause. A simple custom exception "Accessing prop Microsoft.X failed, internals must have changed." will save you a weekend. Been there.
It is not a problem if you develop something internal in Microsoft because you have VS Enterprise license and there is MS Fakes allowing you to mock any static methods, class, etc. Which is not the case for opensource projects though) And knowing MSFT from inside I would bet it is more like 7 hundred different implementations than just 7 😂
Service level dependency injection serves the same purpose with less verbosity. Kind of like how instance methods implicitly carry a reference to “this”. Sometimes it might be useful to define every dependency in the parameter of the method, but come on, do you really want to explicitly send every dependency in every single method? Only if you are a FP purist
It always amuses me that everyone loses their minds when it comes to times/dates/timezones etc. The concepts are pretty well defined, and have been implemented successfully in 3rd party libraries (ie NodaTime) for years. Every iteration from MS makes things worse!!
Time is a unit. Clock is a utility (one of many) to measure time. I feel Nick some too many times like trolling over minor things that are subjective. 🤔 Less of that please.
1:12 "it's only 1 time at a given time"
Great insight from the C# philosopher.
As someone who works with both hardware and software, I'm going to agree with their decisions. "Clock" has different meanings in hardware - does it provide a tick or a time? ITimeProvider makes that abundantly clear. AS for the second, Properties inherently signal that no work is done when retrieving the value, but there are definitely situations where that's not true. What if I'm using an NtpTimeProvider or some other provider that does some real work, that could take real time or resources to retrieve? A method makes that way, way more clear IMO
I especially love the comment here about engaging with the team to voice opinion. Be vocal about what you think! One minor change I would add though - make sure the discourse is respectful and calm, otherwise nobody will listen.
I've been very vocal urging Microsoft to discontinue Azure Logic Apps but it hasn't happened yet unfortunately :(
There are also clock speeds and other hardware related things, but 2:56 is still the best part of this video :D
I think the reason they made it an abstract class is that that's what their internal guidelines say to prefer -- because newer versions of the framework can add new properties to a class, but not to an interface... Obviously they wrote that back before we had default interface methods! I wonder if they've re-evaluated their guidance since. Still, both are easy to mock, so it mostly comes down to style.
Thanks!
And by the way Clock in any electronic thing means a frequency generator, not a time provider. So, a time provider is the best term for this feature)
TimeProvider is a better name than Clock because the word clock specifically refers to the (hardware) device, not its function.
Furthermore, with "clock" i associate hardware (clock) speeds and not time, programming wise.
i think it should be called DateTimeProvider
I think it should be IDateTimeClockProviderService
I suppose that it is called TimeProvider because is like a provider for the "time" dimension in the "spacetime". Why? Because the TimestampFrequency parameter, the CreateTimer method and because of their implementation of FakeTimeProvider who has also the Advance() method
@@jpsytaccount You're just trolling now.
Right?
IDateTimeProvider would make more sense to me. It doesn't give you the Time(TimeOnly). It gives you the Date and Time
... and Offset.
@@robertnullIWorldClock 😂
IDateTimeAbstractFactoryProviderBuilder....
ISuperMonsterHighlyFlexibleWorldClassSponsoredByChronosTimeProvider
Let go complain to change it :)
“We have something that provides the time! It’s a clock!” 😂
Was looking for this comment 😅
Clock is bad name for me, as someone mentioned in the comments it can interface IDateTimeProvider (reason we working not only witch time but with date also) with two methods that will have prefix in name "Get" and they will be corresponding accordingly to the fields in DateTime instance.
I died when he said, "We have a name for things that provide time - IT'S A CLOCK." Naming. Roasted.
Probably better to assume that DotNet programmers have a good reason for their naming conventions. In the case of ITimeProvider, the name is more fitting for a broader range of uses.
Great changes, but I like the property implementation over the method calls.
Its a Clock!! great moment in the presentation!
"... it's The Clock" - I'm putting that on a tshirt
Kudos to Nick, his viewers and Microsoft for getting this fixed!
Good changes, but why bother developers implementing a abstract class for no particular reason. Therefore it should be an interface with two methods: GetUtcNow() and GetLocalNow() and thats it.
Agreed. Still kinda waiting for solid justification for this NOT being the case.
`Func` problem solved.
I think properties are clearer, and should be used instead of a GetUtcNow() method for concistency with other types, Also an Interface with a Default implementation so I can use a mocking framework and restrict some method / properties being used, e.g. Now that returns the current time on the server should not be called. Maybe add a method where you can pass in your own offset and/or Timezone so you know what Timezone will be returned. I personally don't understand why someone would use anything but UtcNow in a web application, except if the time presented is based on user location. Maybe also address the fact that we have to check what OS ve're on before retrieving a specific Timezone.
I dont understand why method instead of prop
It's a time provider. That means, the time is not an intrinsic property of the object, it has to go somewhere else to look for it, same as an API client or a repository. On the contrary, a DateTime is an object that already contains the date and time information in itself, so those are properties. It's a matter of semantics in the end, a property is also a method behind the scenes, but it does make sense. A method clearly depicts that action is being done.
@@MaQy okey I agree about using method gives intention but syntax for prop looks more familiar and it’s shorter
But if it had to look to an api or other external providers the return type should be Task. I rather go with a property instead of a parameterless sync method
I don't see how the call being synchronous or asynchronous makes any difference here. Are you saying that all parameterless synchronous methods should become properties?
@MaQy For just getting the current date/time, I think properties is just fine and convenient
I would still prefer it to be an interface, but I guess it's an improvement.
The IClock approach is also used by ABP framework
I think it’s what NodaTime does too
Hahahah "It's a clock!", love it
I would prefer interface with default implementstion from ms exposing properties, not methods
Hey Nick a video on how API documentation is done.
GetUtcNow should be a property without a setter, not a method in my opinion.
Maybe I'm just slow on the uptake, but I did not understand the framing of this video at all. "The old implementation caused my unit tests to fail". Why? Was there anything in here that explained why they failed? If so, I missed it. "The new implementation fixed my unit tests". Again, why? I heard a lot of discussion about implementation styles and preferences, but what was the actual problem, and why did the new implementation solve it?
2:57 - "It's clock" ahahah
I have a quite nice Time Provider hanging on my wall right now
I've got one on my wrist, but it's not a clock 😁, also the town cryer just went past 😂. All good time providers though.
IMHO IClock is a name so obvious that may be the reason it wasn't selected, to not break everyone's existing code
i would sort of jokingly argue that the clock is an ui, the real time provider would be the quartz
Taking time as a parameter is still a good approach..
Naming Clock is great until you're a business selling actual Clocks, then you have to remap namespaces everywhere.
Not sure why anyone would use this. Suggest just creating an IClock as you originally did in the rare case where something like this is needed.
for the name issue you can do using Clock = TimeProvider xD
As mentioned in the video, you could just pass the actual time rather than a thing that provides the time. Much simpler, no need to mock it.
Doesn't always work. By the time you pass the time down, there is a delta between it being passed down and it being used, so it's going to be inaccurate
@@nickchapsas If that really is an issue, then you could just pass a Func.
@@ReneWiersmaMusic You know as soon as people know of the TimeProvider, they will inject it everywhere, even if only the current DateTime is needed. So many things could just be a simple func. No dependencies, no interfaces, only exactly what is required.
Been thinking about this lately, seeing some class with lots of injected interfaces where 1 method is called on each of them. It feels like this kind of code can be hard to understand because you don't know which calls are made on those interfaces. I wonder if it would be easier if we supplied only the dependencies that are required, but as Funcs instead of interfaces.
@@z0nx Yes! I call this interface obsession. Probably an artefact of Dependency Injection frameworks.
@@ReneWiersmaMusic "That's just how it works" as you Mock every damn thing. That's also "how it works"
Clock is not a better name. Clock might be a good name if it was about time only. IDateTimeProvider it is
I think the properties are far better, as they increase source compatibility as you stated.
What do you mean by "source compatibility"? My understanding is that a property in the background operates exactly how a method's "GET" function does when returning a non-static value.
@@collynchristopherbrenner3245 I mean it is easier to migrate from code using DateTime if you don’t have to add parens everywhere.
1:34 This is probably an unpopular opinion but I think Microsoft's love for internal types is excessive and detrimental to developers using the open source code, and in general I think the internal keyword is a bad practice that treats developers like babies and in all instances it should be changed to moving the type into a namespace (or otherwise separated) that identifies the type as internal but does not lock it off from legitimate tinkering.
Internal types would pollute intellisense to a point where it would be as bad as javascript, causing naming conflicts everywhere. We still have reflection for the 1:1000 chance your project needs some .NET internals exposed, also when you do that YOU are responsible if it breaks in next update and that is correct.
@@the-niker that's the developers are babies argument. I guess maybe they are.
Yes you can use reflection. What's the difference in developer responsibility when using reflection and using a type clearly marked as internal (but not with the internal keyword specifically)? Reflection just makes it harder to know when things have been broken.
I guess I'd settle for some syntactic sugar that allowed easy internal type usage with some compiler warning.
@@orterves Without reflection your production server will one day break after a random low-priority windows update and the person that inherits the project won't have a clue why. Proper safe reflection can isolate the call and keep the system alive just degraded and report the anomaly to logs with probable cause. A simple custom exception "Accessing prop Microsoft.X failed, internals must have changed." will save you a weekend. Been there.
Nick you gotta update your plugins, you're killing me here
It is not a problem if you develop something internal in Microsoft because you have VS Enterprise license and there is MS Fakes allowing you to mock any static methods, class, etc. Which is not the case for opensource projects though) And knowing MSFT from inside I would bet it is more like 7 hundred different implementations than just 7 😂
They probably didn't use IClock or Clock because EVERYONE is already using that, and it would introduce thousands of breaks. :)
You know how FP guys are always crying about `pure` functions and 'referential transparency'? This is why.
Now if only they could evolve the DI system. "ConformingContainer" == "Low common demoninator".
I love how he dismissingly mentions the method parameter solution, when overriding the TimeProvider is more work to effectively do the same thing.
Service level dependency injection serves the same purpose with less verbosity. Kind of like how instance methods implicitly carry a reference to “this”. Sometimes it might be useful to define every dependency in the parameter of the method, but come on, do you really want to explicitly send every dependency in every single method? Only if you are a FP purist
Maybe “constructor level dependency injection” would be a more precise wording
@@AugustBonds I think you've misunderstood what the method parameter solution was, i.e. passing in the DateTime value.
@@AugustBonds Not every dependency, but passing DateTime value is such an obvious choice. Anything else is pure overthinking.
It always amuses me that everyone loses their minds when it comes to times/dates/timezones etc.
The concepts are pretty well defined, and have been implemented successfully in 3rd party libraries (ie NodaTime) for years.
Every iteration from MS makes things worse!!
Time is a unit. Clock is a utility (one of many) to measure time. I feel Nick some too many times like trolling over minor things that are subjective. 🤔 Less of that please.
Always complain.
Abstract class is also okay, but in my opinion GetUtcNow() must be a property like UtcNow