I have created two fakes for ISongsView and they are quite alike. And adding yet another test will probably force me to create yet another fake. So I would like to merge them in some way. This should however be done without thinking too much about the future since I don't know now what the future might look like.
The test code is just as important as the production code. You should give it the same attention as you give your production code when it comes to keeping it clean. The rules are somewhat different, though, as we shall see later on.
The difference between the two fakes is what happens inside the ShowView() method. In one fake I set a flag indicating that the method has been called upon, and I raise the OnViewClosed event. In the other fake I do nothing.
public class SongsViewFake : ISongsView { // ... public void ShowView() { HasBeenShown = true; ClickWindowCloseButton(); } } public class NonClosingSongsViewFake : ISongsView { // ... public void ShowView() { } }It doesn't matter if I always set the HasBeenShown flag, so that difference is easy to equate. Triggering the event, however, should not be done in the non-closing fake (hence the name). One solution would be to have a flag that indicates whether the view should be closed or not and have an if-clause in the ShowView() method. As you might remember, I don't like using if-clauses, though, since they generally are symptoms of breaking good design principles.
In this case, adding the flag would be a violation of the Open-Closed Principle which, in short, says that you should be able to extend an existing system with new behavior without changing existing code.
The triggering of the OnViewClosed event is but one thing that I might want to do in my fake. If I want a different behavior in my next test, using the flag solution I would probably have to add another flag to my fake. That is, I would have to make modifications to the fake. Now I will use a solution with which I can later use my fake for yet unthought-of purposes (extend it) without making modifications to it.
The fact that I'm talking about having different behaviors in the fake makes me think that, having if-clauses, I'm probably breaking the Single Responsibility Principle as well. How can I have several different behaviors in my class and still only have one reason to change?
So what I do is that I inject the behavior that I want into the fake. I call this behavior a show action, and I inject through a property on the fake. This solution is somewhat similar to what I did previously with the closeAction on Thingy. I start by modifying the SongsViewFake, making it general enough to replace the NonClosingSongsViewFake.
public class SongsViewFake : ISongsView { public event Action OnViewClosed; public bool HasBeenShown { get; private set; } private Action<SongsViewFake> ShowAction { get; set; } public SongsViewFake(Action<SongsViewFake> showAction) { ShowAction = showAction; HasBeenShown = false; } public void ShowView() { HasBeenShown = true; ShowAction( this ); } public void ClickWindowCloseButton() { ... } } [TestFixture] public class SongsFixture { [Test] public void Songs_window_is_displayed_when_the_application_starts() { var view = new SongsViewFake(v => v.ClickWindowCloseButton()); var thingy = new Thingy(view, ac => { }); thingy.ShowView(); Assert.That(view.HasBeenShown, Is.True); } [Test] public void Application_closes_when_songs_window_closes() { var view = new SongsViewFake(v => v.ClickWindowCloseButton()); // ... } // ... }The SongsViewFake receives the ShowAction that should be performed in ShowView() as a parameter to the constructor. I made the ClickWindowCloseButton() method public so that it can be called from the test method.
I really like this change. Before the change, the calls to ClickWindowCloseButton() were hidden inside the fake. The fact that the method is called upon is very important for understanding the tests, and now that it is done explicitly in the test method, the tests are a little bit clearer.
The tests are still green. I can now use the modified fake for the last test and remove the need for the NonClosingSongsViewFake.
[TestFixture] public class SongsFixture { // ... [Test] public void Application_does_not_close_before_songs_window_closes() { var view = new SongsViewFake(v => { }); // ... } }I now use the same view fake for all my tests, so I can delete the NonClosingSongsViewFake. The tests are green, so I check in the changes.
This was not the first time that I applied the Open-Closed Principle in this code. The first time was when adding the first test, when I created an interface for ISongsView and different implementations of that interface for test and production. I could have used an if-clause in Thingy, specifying one behavior when running the tests and another in production. Adding another kind of behavior (although I can't think of what behavior that could be) would require me to modify Thingy, something that I don't have to do now.