Azam has written a post about Ben Hall‘s article “Beginning to Mock Using Rhino Mocks and MbUnit“. The logic in the example is a method that returns an image of the sun in the day and the moon at night. In both Azam’s and Ben Halls examples, the GetImage()method returns the image name
public string GetImage() { int currentHour = DateTime.Now.Hour; if (currentHour > 6 && currentHour < 21) return "sun.jpg"; else return "moon.jpg"; }
Using Interface only Mocking Frameworks will lead to redesigning this in order for the code to be testable and extracting and wrapping ‘DateTime.Now’ to an interface and concrete class:
public interface IDateTime { int GetHour(); } public class DateTimeController : IDateTime { public int GetHour() { return DateTime.Now.Hour; } } public class ImageManagement { public string GetImage(IDateTime time) { int currentHour = time.GetHour(); if (currentHour > 6 && currentHour < 21) return "sun.jpg"; else return "moon.jpg"; } }
Scott Bellware has suggested a better design – and that is to move the ‘is it day or night’ logic from the ImageManagement class to the IDateTime
The calculation of whether it is day or night is an orthogonal concern of the image generation and therefore should be moved out of the GetImage method.
Now our code will look at following:
public interface ITimeService { public bool IsDayTime(DateTime time) } public class ConcreteTimeService : ITimeService { public int IsDayTime(DateTime time) { int currentHour = time.GetHour(); return (currentHour > 6 && currentHour < 21) ; } } public class ImageManagement { public static string GetImage(ITimeService time) { if (time.IsDayTime(DateTime.Now)) return "sun.jpg"; else return "moon.jpg"; } }
The first thing to note is that being forced to use interfaces and Dependency Injection (the first example) did not naturally lead to a good design – there is still a need to think about the design in classical – old fashion – Object Oriented terms.
But if we are thinking about designing in the old fashioned way, here is another approach to reach the same (or better) results: using extension methods, which is a much more natural way of doing this in .NET 3.0+
public static class DateTimeExtensions { public static bool IsDayTime(this DateTime time) { int currentHour = time.GetHour(); return (currentHour > 6 && currentHour < 21) ; } } public class ImageManagement { public static string GetImage() { if (DateTime.Now.IsDayTime()) return "sun.jpg"; else return "moon.jpg"; } }
This makes more sense as we are extending the abilities of DateTime, the code is easy to read and understand. The only problem is how do we test this code.
Well it is simple with Typemock Isolator here is how:
[Test,ClearMocks] public void WhenItIsDayTime_ReturnSunImage() { // pretend it is always day using (RecordExpectations r = RecorderManager.StartRecording()) { r.ExpectAndReturn( DateTime.Now.IsDayTime, true); } Assert.AreEqual("sun.jpg", ImageManagement.GetImage()); }
So here are two points to think about:
- You must always think about your design, using IoC and DI does not automatically lead to better designs
- If you are already thinking about your design, you might as well have the freedom to develop the design you reached, without limitations of your tools
Brilliant post Eli. I’m a big believer in IoC, etc., helping lead the way toward a better design. But you’re post showed that no matter how many tools and techniques you employ, you still have to actively *think* about design. The tools and techniques will only get us so far.
Hmm, I am not sure about this, and I don’t think using TypeMock should be encouraged for this sort of solution. I am seeing TypeMock used on more and more projects to hide bad/lack of design, when it was really built to enable testing of legacy components which cannot be refactored.
Still… I believe the whole assumption that a mock is required for any of this is incorrect. Why do you need to mock in the first place? If your ImageManagement.GetImage function simply accepts a DateTime as a dependency then you ahve removed the dependency on DateTime.Now. Then you can test your method passing in a DateTime where the hour is less than 6pm and one where it is greater than 6pm.
Why assume that your consumer will always be interested in determining whether it is currently day or night? For example, he may wish to perform a lookup on historical data. You are making your service less useful by forcing the dependency on DateTime.Now.
Thanks Chris
Neil, Typemock is used on many projects that have GOOD – Well thought designs, albeit not bound to using ONLY the IoC Pattern.
These designs are a result of hard *thinking* in OO terms. Just because a developer doesn’t have the correct tools to verify that his code is working, doesn’t make his design bad or lacking.
About your design point: It is a valid point for this example, but not for the concept that I am trying to convey.
See, accepting DateTime as an argument is defying the encapsulation principal and perhaps the YAGNI principal too.
Why should the client care about how we get the correct image? Why should he know that we are even asking for the time?
Hi,
Recently I discovered that perhaps dependency injection is a bit overused. Sometimes it’s quite frustrating: you want to use someone else’s class, and you have to figure out a lot of dependencies only to find out that the 6th in the chain is internal or something.
Anyway, you could have two completely independent classes here with two methods:
bool DayService.IsDayTime(this DateTime time)
and
string ImageProvider.GetImage(bool gimmeTheDayImage)
Both classes can be tested without any mocks, and both can be reused in other places without any knowledge of each other, or any related interfaces.
We still need a high-level API though, a class that glues these classes. How do we test it? We write an integration test. And that’s where TypeMock can really help, since we can’t use DI at this level. Too bad we can’t mock mscorlib..
Thanks ulu,
You are correct, This stresses the point that there is a need to *think* about the API, and not let the tests lead to wrong or over designed software.
In any case we might find it better to encapsulate the fact that GetImage() changes the image according to the time, and we might not want our client to pass a boolean time, we might in the future want to change the image every hour, or depending on geo-location. These are implementation details that should be encapsulated.