Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Recently I had someone describe a "Time Machine" service to me; that is, a service that could be used to change how a chunk of code perceives time.

Normally through calls to DateTime.Now and other DateTime statics, time is directly tied to the system clock. A Time Machine service is used instead of the static DateTime calls, and allows external code to change the time it reports. This is especially useful when writing unit tests for services that behave different as time passed due to timers, triggers, etc.

I liked the idea, and decided to implement my own version. Here are the relevant interfaces:

// A service that can travel through time
public interface ITimeTraveler
{
    ITimeMachine TimeMachine { get; set; }
}

// Interface for a Time Machine
public interface ITimeMachine
{
    DateTime Now();
    DateTime UtcNow();
    DateTime Today();
    DateTime UtcToday();
    void TimeTravel(TimeSpan adjustment);
    void TimeTravelTo(DateTime newDateTime);
    void FreezeTime();
    void UnfreezeTime();
    void RevertAllTimeTravel();
    bool IsCurrentlyTimeTraveling();
}

Here is the Time Machine Gist with the full implementation.


This includes two revisions. The first revision was the code I linked to originally, and additional revisions take the feedback here into consideration.

I'm looking for any feedback or suggestions on both the concept of this service and my current implementation.

share|improve this question
9  
Just clicked the link. 137 lines of code, too long? Meh, we review twice as much for breakfast! Feel free to put it all in and have the implementation reviewed as well! –  Mat's Mug Apr 25 at 2:58
    
@Mat'sMug I have added the full implementation as you requested! I'm not yet familiar with what is acceptable to paste in versus link to. –  jmblack Apr 25 at 12:27
2  
Awesome! It's nice that you kept the original reviewed code in place; please see these guidelines for everything this community agrees makes sense in such situations ;) –  Mat's Mug Apr 25 at 13:26
4  
As @Mat'sMug noted, the question should not be updated with new code. Fortunately, Code Review has a time machine service! I have rolled back Rev 4 to Rev 2. Please follow the advice in the guidelines instead. Thanks for your cooperation. –  200_success Apr 25 at 15:00
2  
Thanks, I'll update the text/links in this version to explain what's what without changing the code, and as mentioned before I'll follow those guides now that I've seen them! –  jmblack Apr 25 at 15:02

3 Answers 3

up vote 4 down vote accepted

I like it.

For some very specific cases where a class needs to be tested and its functionality depends on what System.DateTime.Now returns, then you need an abstraction to wrap the static method calls, if you want to be able to write unit tests that have full control over that dependency.

Your ITimeMachine does that nicely. While a meaningful name, IDateTimeService seems more appropriately generic though. ITimeTraveler might be superfluous.

The dependent code would look like this:

public class MyClass
{
    private readonly IDateTimeService _service;

    public MyClass(IDateTimeService service)
    {
        _service = service;
    }

    public void DoSomething()
    {
        var now = _service.Now(); // instead of DateTime.Now;
        //...
    }
}

I also like that you made Now() a method. It abstracts away the fact that DateTime.Now is really a method in a property suit.

share|improve this answer
1  
Perhaps a use case could be testing particularly weird date issues very deeply into an instance, before determining the exact cause? –  grovesNL Apr 25 at 3:20
1  
Once I incorporated ChrisW's feedback, the naming you suggested works perfectly with the new interface/classes. –  jmblack Apr 25 at 12:30

My guess is that:

  • These methods are used by the code being tested:

    DateTime Now();
    DateTime UtcNow();
    DateTime Today();
    DateTime UtcToday();
    
  • These methods are not called by the code being tested, and are instead called by the unit-test code (to set the 'now' experienced by the code being tested)

    void TimeTravel(TimeSpan adjustment);
    void TimeTravelTo(DateTime newDateTime);
    void FreezeTime();
    void UnfreezeTime();
    void RevertAllTimeTravel();
    bool IsCurrentlyTimeTraveling();
    

The latter methods therefore needn't be in this interface (which is passed to the code being tested). These methods could instead be in a separate interface (known only to the unit-test code), or they could simply be methods of the TimeMachine class.

share|improve this answer
    
This separation seems natural and it also prevents the extra logic and work from the Time Machine methods from being called in non-test code. I have incorporated this change into my OP. Thanks! –  jmblack Apr 25 at 12:29

Implementation

My other answer only covered the original interface(s). This one highlights a point I've noticed in the implementation.

/// <summary>
/// Retrieve the current date and time.
/// </summary>
public DateTime Now()
{
    return FrozenDateTime != null 
        ? FrozenDateTime.Value 
        : DateTime.Now.Add(Offset);
}

/// <summary>
/// Move to the specific point in time provided.
/// </summary>
/// <param name="newDateTime">The point in time to move to.</param>
public void TimeTravelTo(DateTime newDateTime)
{
    Offset = newDateTime.Subtract(DateTime.Now);
}

I was curious to see it in action, so I wrote this little test:

class Program
{
    static void Main(string[] args)
    {
        var service = new TimeMachine();
        Console.WriteLine("Current time is: {0} (system time is: {1})", service.Now(), DateTime.Now);

        //service.FreezeTime();
        //Console.WriteLine("Time frozen at: {0}", service.Now());

        Thread.Sleep(2000);
        Console.WriteLine("Slept for 2000ms.");

        Console.WriteLine("Current time is: {0} (system time is: {1})", service.Now(), DateTime.Now);

        service.TimeTravelTo(DateTime.Now.AddHours(1));
        Console.WriteLine("Time-traveled 1 hour into the future.");

        Console.WriteLine("Current time is: {0} (system time is: {1})", service.Now(), DateTime.Now);

        service.UnfreezeTime();
        Console.WriteLine("Time unfrozen.");

        Console.WriteLine("Current time is: {0} (system time is: {1})", service.Now(), DateTime.Now);
        Console.ReadLine();
    }
}

Output:

enter image description here

If I uncomment the FreezeTime() call, I get this:

enter image description here

This looks like a bug to me, because it's as if the Sleep(2000) never happened.

If I change the implementation of TimeTravelTo() to use the class' own abstraction of Now:

/// <summary>
/// Move to the specific point in time provided.
/// </summary>
/// <param name="newDateTime">The point in time to move to.</param>
public void TimeTravelTo(DateTime newDateTime)
{
    Offset = newDateTime.Subtract(Now());
}

I get this:

enter image description here

Which makes more sense to me (as long as time is frozen, Sleep(2000) has no impact, but when you unfreeze time the offset gets back on track) - but I might be wrong and your implementation could very well be doing exactly what it's supposed to be doing. Hence, my point isn't necessarily that there's a bug in your code, but that your XML documentation should be clearer about this behavior: is unfreezing time supposed to catch up with the actual date/time?

share|improve this answer
1  
The XML is a bit unclear here. Unfreezing is meant to only let the passage of time resume; there is RevertAllTimeTravel() for reverting to the actual present. I wanted to keep these separate functions so that the user has options. –  jmblack Apr 25 at 14:28

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.