17

I'm looking for guidelines to using a consistent value of the current date and time throughout a transaction.

By transaction I loosely mean an application service method, such methods usually execute a single SQL transaction, at least in my applications.

Ambient Context

One approach described in answers to this question is to put the current date in an ambient context, e.g. DateTimeProvider, and use that instead of DateTime.UtcNow everywhere.

However the purpose of this approach is only to make the design unit-testable, whereas I also want to prevent errors caused by unnecessary multiple querying into DateTime.UtcNow, an example of which is this:

// In an entity constructor:
this.CreatedAt = DateTime.UtcNow;
this.ModifiedAt = DateTime.UtcNow;

This code creates an entity with slightly differing created and modified dates, whereas one expects these properties to be equal right after the entity was created.

Also, an ambient context is difficult to implement correctly in a web application, so I've come up with an alternative approach:

Method Injection + DeterministicTimeProvider

  • The DeterministicTimeProvider class is registered as an "instance per lifetime scope" AKA "instance per HTTP request in a web app" dependency.
  • It is constructor-injected to an application service and passed into constructors and methods of entities.
  • The IDateTimeProvider.UtcNow method is used instead of the usual DateTime.UtcNow / DateTimeOffset.UtcNow everywhere to get the current date and time.

Here is the implementation:

/// <summary>
/// Provides the current date and time.
/// The provided value is fixed when it is requested for the first time.
/// </summary>
public class DeterministicTimeProvider: IDateTimeProvider
{
    private readonly Lazy<DateTimeOffset> _lazyUtcNow =
        new Lazy<DateTimeOffset>(() => DateTimeOffset.UtcNow);

    /// <summary>
    /// Gets the current date and time in the UTC time zone.
    /// </summary>
    public DateTimeOffset UtcNow => _lazyUtcNow.Value;
}

Is this a good approach? What are the disadvantages? Are there better alternatives?

Gebb
  • 6,371
  • 3
  • 44
  • 56
  • It seems obvious that the solution is to have a singleton/lazy stuck to your ambient context whatever that context is (not even mentioning DI that is not part of your title nor initial hypothesis). In ASP.NET w/o DI, you could perfectly tie this singleton to the current request (HttpContext.Items Property) if the context is the request (in this case you can even initialize the Lazy w/o thread synchronization like you do in your code sample) – Simon Mourier Sep 27 '15 at 07:35
  • @SimonMourier: Let's say I put it in a singleton and use it as an ambient context. I initialize UtcNow of the singleton to be a callback that gets the current time from the storage tied to the HTTP request. That would work, I guess, but to me method injection looks slightly better because of more intention-revealing signatures of entities' methods. When you look at the unit tests of the singleton-based code, you see a preparation of a seemingly unrealted class before a call to SUT, not ovious why, whereas with method injection you just arrange a required parameter. – Gebb Sep 27 '15 at 10:52
  • This is a good idea. I'm doing the same thing. It makes debugging a lot easier when you can tell just from the time stamp what values were written at the same time. – usr Oct 01 '15 at 12:07

4 Answers4

8

Sorry for the logical fallacy of appeal to authority here, but this is rather interesting:

John Carmack once said:

There are four principle inputs to a game: keystrokes, mouse moves, network packets, and time. (If you don't consider time an input value, think about it until you do -- it is an important concept)"

Source: John Carmack's .plan posts from 1998 (scribd)

(I have always found this quote highly amusing, because the suggestion that if something does not seem right to you, you should think of it really hard until it seems right, is something that only a major geek would say.)

So, here is an idea: consider time as an input. It is probably not included in the xml that makes up the web service request, (you wouldn't want it to anyway,) but in the handler where you convert the xml to an actual request object, obtain the current time and make it part of your request object.

So, as the request object is being passed around your system during the course of processing the transaction, the time to be considered as "the current time" can always be found within the request. So, it is not "the current time" anymore, it is the request time. (The fact that it will be one and the same, or very close to one and the same, is completely irrelevant.)

This way, testing also becomes even easier: you don't have to mock the time provider interface, the time is always in the input parameters.

Also, this way, other fun things become possible, for example servicing requests to be applied retroactively, at a moment in time which is completely unrelated to the actual current moment in time. Think of the possibilities. (Picture of bob squarepants-with-a-rainbow goes here.)

Mike Nakis
  • 56,297
  • 11
  • 110
  • 142
  • That is a good alternative. One advantage is that fewer types are involved. And the meaning can be conveyed by the parameter name. A disadvantage is probably that it makes it easier to pass not the current method's `requestTime` down the line but some made-up value, but if the parameter is named the same throughout the domain, such a mistake would be difficult to make. – Gebb Oct 01 '15 at 22:56
  • 2
    I am not sure whether it was clear from my wording, but I did not post this here as a simply a good alternative; I posted it as the solution which makes most sense *by far*. The *current time* is a notion which I very often see being misused by programmers. Most times a programmer *thinks* that they need the current time, what they in fact need is the time of the operation which is being performed. Knowledge of the fact that an operation's time should be the actual real time should be isolated in as few places as possible. The need to mock the clock is usually indicative of a misuse of time. – Mike Nakis Oct 02 '15 at 05:45
  • 1
    Makes sense, Mike, thank you very much for the insight. – Gebb Oct 02 '15 at 11:07
  • 1
    I like this approach as well. :) – Matt Johnson-Pint Oct 03 '15 at 06:32
4

Hmmm.. this feels like a better question for CodeReview.SE than for StackOverflow, but sure - I'll bite.

Is this a good approach?

If used correctly, in the scenario you described, this approach is reasonable. It achieves the two stated goals:

  1. Making your code more testable. This is a common pattern I call "Mock the Clock", and is found in many well-designed apps.

  2. Locking the time to a single value. This is less common, but your code does achieve that goal.

What are the disadvantages?

  1. Since you are creating another new object for each request, it will create a mild amount of additional memory usage and additional work for the garbage collector. This is somewhat of a moot point since this is usually how it goes for all objects with per-request lifetime, including the controllers.

  2. There is a tiny fraction of time being added before you take the reading from the clock, caused by the additional work being done in loading the object and from doing lazy loading. It's negligible though - probably on the order of a few milliseconds.

  3. Since the value is locked down, there's always the risk that you (or another developer who uses your code) might introduce a subtle bug by forgetting that the value won't change until the next request. You might consider a different naming convention. For example, instead of "now", call it "requestRecievedTime" or something like that.

  4. Similar to the previous item, there's also the risk that your provider might be loaded with the wrong lifecycle. You might use it in a new project and forget to set the instancing, loading it up as a singleton. Then the values are locked down for all requests. There's not much you can do to enforce this, so be sure to comment it well. The <summary> tag is a good place.

  5. You may find you need the current time in a scenario where constructor injection isn't possible - such as a static method. You'll either have to refactor to use instance methods, or will have to pass either the time or the time-provider as a parameter into the static method.

Are there better alternatives?

Yes, see Mike's answer.

You might also consider Noda Time, which has a similar concept built in, via the IClock interface, and the SystemClock and FakeClock implementations. However, both of those implementations are designed to be singletons. They help with testing, but they don't achieve your second goal of locking the time down to a single value per request. You could always write an implementation that does that though.

Community
  • 1
  • 1
Matt Johnson-Pint
  • 230,703
  • 74
  • 448
  • 575
  • Thanks for the answer. Regarding posting it to the CodeReview site: is it possible that the question would receive the same amount of attention there? – Gebb Oct 02 '15 at 11:21
  • @Gebb It's not necessary (or desirable) to "cross post" the same question to multiple SE sites. This has received enough attention here that I'd say it's a moot point, but consider Code Review next time for *complete, functioning* code. – Air Oct 02 '15 at 20:51
  • 1
    @Air I meant "... if I posted it there". I did not intend to crosspost. My question was about whether it is as effective to ask on Code Review as is here (in terms of received attention). – Gebb Oct 02 '15 at 21:56
  • 1
    @Gebb That's difficult to answer. Sometimes questions get more or less attention and there's no clear reason why. Code Review is quite an active site, though; [its most popular new questions in the past month](http://codereview.stackexchange.com/?tab=month) have received nearly as many answers and about half as many views, compared with [the same sample from SO](http://stackoverflow.com/questions/popular?show=month&sort=votes). – Air Oct 02 '15 at 22:27
2

Code looks reasonable.

Drawback - most likely lifetime of the object will be controlled by DI container and hence user of the provider can't be sure that it always be configured correctly (per-invocation and not any longer lifetime like app/singleton).

If you have type representing "transaction" it may be better to put "Started" time there instead.

Alexei Levenkov
  • 98,904
  • 14
  • 127
  • 179
0

This isn't something that can be answered with a realtime clock and a query, or by testing. The developer may have figured out some obscure way of reaching the underlying library call...

So don't do that. Dependency injection also won't save you here; the issue is that you want a standard pattern for time at the start of the 'session.'

In my view, the fundamental problem is that you are expressing an idea, and looking for a mechanism for that. The right mechanism is to name it, and say what you mean in the name, and then set it only once. readonly is a good way to handle setting this only once in the constructor, and lets the compiler and runtime enforce what you mean which is that it is set only once.

// In an entity constructor:
this.CreatedAt = DateTime.UtcNow;
  • The `readonly` approach works well if you work with only one object which requires the current time. But the question is about a "transaction consistent date". There is no obvious single place where you could store that readonly current date which can be used by any object throughout the transaction. – Monsignor Oct 12 '15 at 08:44