Implementing the last test left me with some duplication. Here is the copy found in the production code that I want to extract into a reusable method.
public partial class SongsView : Form, ISongsView { public event Action OnViewClosed; // ... private void SongsViewFormClosed( object sender, FormClosedEventArgs e) { var handler = OnViewClosed; if (handler == null ) throw new InvalidOperationException("No handler registered for the event ."); handler(); } }This is a refactoring that probably all of today's IDE:s have built-in: extract method. I just extract the method from the last two lines into a method called Raise(). The method does not access the instance, so I also make it static.
private void SongsViewFormClosed( object sender, FormClosedEventArgs e) { var handler = OnViewClosed; Raise(handler); } private static void Raise(Action handler) { if (handler == null ) throw new InvalidOperationException("No handler registered for the event ."); handler(); }Now I would like to use the same method from the test code. This suggests that the method does not belong to SongsView, but rather in some shared location.
Since it is a static method, I could convert it into a non-static method that belongs to the type of the parameter, Action. However, Action is a delegate which can't have methods on it, and it is anyway not something that I can modify since it is part of the .NET framework. So I will instead convert the method into an extension method on Action.
Extension methods are a really powerful feature that is not found in java. They enable you to add methods to an existing type without modifying that type. That is, to the caller it looks like it is a normal instance method while it is actually defined elsewhere.
This allows you to add functionality to types that you don't own, and also add functionality to types that can't have implementations, such as delegates and interfaces.
While extension methods look the same to the caller, they are different from normal instance methods in the sense that they don't have access to non-public members.
First, I create an empty static class ActionExtensions:
public static class ActionExtensions { }Then I use my refactoring tool's "Move To Another Type" on the method and specify that I want to move it to the ActionExtensions class. Apart from moving the method, all references to the method are updated to point to the new location. At the same time, I make the method public so that it is accessible from the caller.
public partial class SongsView : Form, ISongsView { // ... private void SongsViewFormClosed( object sender, FormClosedEventArgs e) { var handler = OnViewClosed; ActionExtensions.Raise(handler); } } public static class ActionExtensions { public static void Raise(Action handler) { if (handler == null ) throw new InvalidOperationException("No handler registered for the event ."); handler(); } }Finally, I convert the Raise() method into an extension method by adding a this modifier on the first parameter, and also modify the caller to use it as such:
public partial class SongsView : Form, ISongsView { // ... private void SongsViewFormClosed( object sender, FormClosedEventArgs e) { OnViewClosed.Raise(); } } public static class ActionExtensions { public static void Raise( this Action handler) { // ... } }By moving the method to an extension method I have accomplished two things. One is that I have found a nice home for a shared method that could otherwise be hard to find a good home for. The other is that the code raising the event now looks really slick and easy to read.
Finally, I change the other copy of the event raising code to use the same extension method:
public class SongsViewFake : ISongsView { // ... private void ClickWindowCloseButton() { OnViewClosed.Raise(); } }Really nice.
I do one more thing with the extension method, and that is that I rename the parameter from handler to me:
public static class ActionExtensions { public static void Raise( this Action me) { ... } }The reason is that this parameter is a bit special. It corresponds to this in a normal instance method, and I use the convention of always naming it me to remind me of that.
A reflection I make is that now that the event raising logic is in a separate method, it is also much easier to test that InvalidOperaionException is thrown when there are no event subscribers. I could easily write a unit test that only executes the ActionExtensions.Raise() method.
For now, I choose not to do that though. When the check was part of the SongsView I felt a little bit nervous about it. The check felt misplaced and was surrounded by other, unrelated things. Now that it is extracted into its own class and method I feel much safer. The chances that someone would get confused about the code and accidently break it are small. So though improved design, I feel I have also reduced the need for full test coverage.
The tests are all green, and running the application works fine, so I check in my changes.