Extension Methods
Extension methods are a really cool new feature of C# that were introduced in version 3.0 of the language. Essentially they are static methods that act like instance methods, allowing you to extend objects you don't own. Ok, seeing those words on screen makes me think this is a terrible idea, but it really does have it's place. (LINQ relies heavily on it)When I first saw these I got quite excited because we had a number of scenarios where they would make our code much much cleaner and easier to maintain. I can name a couple examples in our own project where this has the potential to make the API cleaner.
Example 1: Helper/Utility/Static Methods
Try as they might, the .NET framework guys will not anticipate every piece of code that ends up repeated hundreds of times across your project to get around a common case. Before the framework added String.IsNullOrEmpty() in 2.0 we had StringHelper.IsNullOrEmpty(string arg)in our code base along with about half a dozen other methods. Now string may not be the best example, because in my mind I think it's a bad idea to write extensions to framework types, but it does illustrate the problem well.In our project we have maybe a dozen of these Helper classes full of static methods (utility pattern). They are useful, but the biggest problem is the team's ability to consistently discover those methods. Emails and other forms of communication helps, code review helps but ultimately you end up with repeated code fragments where helpers could have been used or even worse you end up with competing helpers in different namespaces that need to be consolidated once discovered.
Example 2: Enhancing Functionality Based on Context
Another useful place for extension methods in our project is to have our domain model be extended differently based on the area of the application and what rights the users executing that code have. Essentially what we have is two sets of functionality that are implemented very differently based on the context. For example in one context our object model is mapped using an ORM to the database directly, where as in another context that same object model is used with pre-populated data sets that are cached and completely scriptable by our end users. We still have code that needs to understand these objects across this context however, leading us to create interfaces for every single object in that domain that needs to work across these two contexts. I'm a fan of interfaces, but I think in this scenario we've clearly lost something in terms of the DRY principle and code readability.I've yet to really map out what this will look like for our project, but I see what's been done with Linq and am excited about how simple it could be for example to include the ".Scripting" namespace to attach methods to our domain model that are exposed to end users. Similarly an ".API" namespace for our internal privileged code base with everyone sharing the same core objects.
The Gotchya that kept me at work an extra hour
Symptoms- Your extension method is no longer ever being executed
- Intellisense shows the correct method signature, reports no problems
- Right click "go to definition" takes you to the method you think will be hit
- Stepping through the code shows you never reach your extension method
I've created some fake code to illustrate the problem. Imagine we have a simple factory class providing some useful functions like so :
namespace ExtensionMethodsTest
{
public class FactoryA
{
public ObjectA GetInstance()
{
return new ObjectA("empty object", -1);
}
public ObjectA GetInstance(string name)
{
return new ObjectA(name, 0);
}
}
}
But then we decide that we actually need to grab instances of ObjectA using an int id, so we add that using an extension method like so :
using ExtensionMethodsTest;
namespace MyExtendingNamespace
{
// a container for my extensions
public static class ExtendFactoryA
{
// Extend our factory to look objects up by id
public static ObjectA GetInstance(this FactoryA factory, int id)
{
return new ObjectA("got by id", id);
}
}
}
Blamo! Now anytime we're using the "MyExtendingNamespace" our FactoryA includes the third way to grab an instance of ObjectA.
Here is how I am calling both of these :
private void ByIdButton_Click(object sender, EventArgs e)
{
FactoryA fa = new FactoryA();
ObjectA a = fa.GetInstance(42); // call extension method with int
this.label1.Text = a.ToString();
}
private void ByNameButton_Click(object sender, EventArgs e)
{
FactoryA fa = new FactoryA();
ObjectA a = fa.GetInstance("gotten by name"); // call instance method with string
this.label1.Text = a.ToString();
}
This seems ok, our calls to GetInstance are consistent and we have everything working. Now transplant yourself a couple months into the future when the original owner of the FactoryA class has a need to alter the behavior to allow for more scenarios for retrieving instances of ObjectA.
public ObjectA GetInstance(object proxyObj)
{
return new ObjectA(proxyObj.GetType().Name, 0);
}
Now from the calling code everything still appears to work fine, intellisense continues to show the "int" signature that you think you are calling, and in fact if you right click the GetInstance call and say "Go to Definition" it takes you to the MyExtendingNamespace version of GetInstance.
At runtime however the CLR looks first for something that matches your call to fa.GetInstance(42); within the main namespace/class before looking at any extensions. Because our int gets automatically boxed we actually match "object proxyObject" and will never reach the extension method. Depending on the specific implementation in your code this can be a particularly insidious error or it may fail outright. Either way on a large project it can be a real annoyance to track down.
To me there are a few mistakes here. 1) Why use an extension method here at all? 2) In general I think extension methods as overrides make little sense given how the CLR matches these and 3) GetInstance(object proxyObject) should probably be something more specific like ProxyBase or something else that is not an object. (Akin to catching System.Exception, bad... be specific)
Fixing this issue is as simple as renaming the extension method to GetInstanceById, or moving the method onto the Factory itself, or fixing the factory method to not use object. Personally I say drop the extension method.
My example is over simplified, and in our scenario at work there was a little more justification for not changing the class that defined the original functionality. To me though this seems like the tip of an iceberg of problems.
- Why negotiate with a module maintainer when I can just add functionality right here right now from my own code?
- Why stop to understand that class and how it's been constructed when I can just impose my view of how it should work overtop?
- Intellisense makes it easy to find the method, so I don't have to worry about how I organize this code.
Terrifying.