I've been thinking a lot about extension methods lately, and I must admit I both love them and hate them. They are a lot like sugar, they taste so nice and sweet, but they'll rot your teeth if you eat them too much.
I can't deny that they aren't useful and very handy. One of the major components of the Shared Component library where I work is a set of useful extension methods. But, I also can't deny that they tend to be overused and abused to willy-nilly extend every living type.
So what constitutes a good extension method? Obviously, you can write an extension method for nearly anything whether it is a good idea or not. Many times, in fact, an idea seems like a good extension method but in retrospect really doesn't fit.
So what's the litmus test? To me, an extension method should be like in the movies when a person runs into their twin, separated at birth. You just know you're related. Obviously, that's hard to quantify, so let's try to put a few rules-of-thumb around them.
A good extension method should:
Apply to any possible instance of the type it extends.
Simplify logic and improve readability/maintainability.
Apply to the most specific type or interface applicable.
Be isolated in a namespace so that it does not pollute IntelliSense.
So let's look at a few examples in relation to these rules.
The first rule, to me, is the most important of all. Once again, it bears repeating, a good extension method should apply to all possible instances of the type it extends. It should feel like the long lost relative that should have been included in the original class but somehow was missing from the family tree.
Take this nifty little int extension, I saw this once in a blog and at first I really thought it was pretty cool, but then I started noticing a code smell I couldn't quite put my finger on. So let's look:
public static class IntExtensinos
{
public static int Seconds(int num)
{
return num * 1000;
}
public static int Minutes(int num)
{
return num * 60000;
}
}
This is so you could do things like:
...
Thread.Sleep(5.Seconds());
...
proxy.Timeout = 1.Minutes();
...
Awww, you say, that's cute! Well, that's the problem, it's kitschy and it doesn't always apply (and incidentally you could achieve the same thing with TimeStamp.FromSeconds(5)). It's syntactical candy that looks cool, but tends to rot and pollute the code. It would allow things like:
total += numberOfTodaysOrders.Seconds();
which makes no sense and should never be allowed. The problem is you're applying an extension method to a logical domain, not a type domain. That is, the extension method Seconds() doesn't really apply to ALL ints, it applies to ints that are representative of time that you want to convert to milliseconds.
Do you see what I mean? The two problems, in a nutshell, are that a) Seconds() called off a non-time value makes no sense and b) calling Seconds() off something to pass to something that does not take milliseconds will be off by a factor of 1000 or worse.
Thus, in my mind, you should only ever have an extension method that applies to the whole domain of that type.
For example, this is one of my personal favorites:
public static bool IsBetween<T>(this T value, T low, T high)
where T : IComparable<T>
{
return value.CompareTo(low) >= 0 && value.CompareTo(high) <= 0;
}
This allows you to check if any IComparable<T> is within an upper and lower bound. Think of how many times you type something like:
if (response.Employee.Address.YearsAt >= 2
&& response.Employee.Address.YearsAt <= 10)
{
...
}
Now, you can instead type:
if(response.Employee.Address.YearsAt.IsBetween(2, 10))
{
...
}
Note that this applies to all IComparable<T> -- that's ints, chars, strings, DateTime, etc -- and does not depend on any logical domain. In addition, it satisfies the second point and actually makes the code more readable and maintainable.
Let's look at the third point. In it we said that an extension method should fit the most specific interface or type possible. Now, I'm not saying if you have something that applies to enumerables, you create an extension for List, Array, Dictionary, etc (though you may have reasons for doing so), but that you should beware of making things TOO general.
For example, let's say we had an extension method like this:
public static T ConvertTo<T>(this object value)
{
return (T)Convert.ChangeType(value, typeof(T));
}
This lets you do more fluent conversions like:
double d = "5.0".ConvertTo<double>();
However, if you dig into Reflector (LOVE that tool) you will see that if the type you are calling on does not implement IConvertible, what you convert to MUST be the exact type or it will throw an InvalidCastException. Now this may or may not be what you want in this situation, and I leave that up to you. Things like this would fail:
object value = new Employee();
...
// class cast exception because typeof(IEmployee) != typeof(Employee)
IEmployee emp = value.ConvertTo<IEmployee>();
Yes, that's a downfall of working with Convertible in general, but if you wanted your fluent interface to be more type-safe so that ConvertTo were only callable on IConvertibles (and let casting be a manual task), you could easily make it:
public static T ConvertTo<T>(this IConvertible value)
{
return (T)Convert.ChangeType(value, typeof(T));
}
This is what I mean by choosing the best type to extend. Consider that if we used the previous (object) version, every time we typed a dot ('.') on an instance we'd pull up ConvertTo() whether it was applicable or not. By filtering our extension method down to only valid types (those that implement IConvertible) we greatly reduce our IntelliSense pollution and apply a good level of compile-time correctness.
Now my fourth rule is just my general rule-of-thumb. Obviously, you can make extension methods as in-your-face as you want. I included all mine in my work libraries in its own sub-namespace, something akin to:
namespace Shared.Core.Extensions { ... }
This is in a library called Shared.Core, so just referencing the Core library doesn't pollute your IntelliSense, you have to actually do a using on Shared.Core.Extensions to bring the methods in. This is very similar to the way Microsoft puts its extension methods in System.Linq. This way, if you want 'em, you use the appropriate namespace. If you don't want 'em, they won't pollute your namespace.
To really make this work, however, that namespace should only include extension methods and subordinate types those extensions themselves may use. If you plant other useful classes in those namespaces, once a user includes it, they get all the extensions too.
Also, just as a personal preference, extension methods that aren't simply syntactical shortcuts, I like to put in a static utility class and then have extension methods for syntactical candy. For instance, I think it imaginable that any object could be converted to XML:
namespace Shared.Core
{
// A collection of XML Utility classes
public static class XmlUtility
{
...
// Serialize an object into an xml string
public static string ToXml(object input)
{
var xs = new XmlSerializer(input.GetType());
// use new UTF8Encoding here, not Encoding.UTF8. The later includes
// the BOM which screws up subsequent reads, the former does not.
using (var memoryStream = new MemoryStream())
using (var xmlTextWriter = new XmlTextWriter(memoryStream, new UTF8Encoding()))
{
xs.Serialize(xmlTextWriter, input);
return Encoding.UTF8.GetString(memoryStream.ToArray());
}
}
...
}
}
I also wanted to be able to call this from an object like:
value.ToXml();
But here's the problem, if i made this an extension method from the start with that one little keyword "this", it would pop into IntelliSense for all
objects which could be very polluting. Instead, I put the logic into a utility class so that users have the choice of whether or not they
want to use it as just a class and not pollute IntelliSense, then in my extensions namespace, I add the syntactical candy:
namespace Shared.Core.Extensions
{
public static class XmlExtensions
{
public static string ToXml(this object value)
{
return XmlUtility.ToXml(value);
}
}
}
So now it's the best of both worlds. On one hand, they can use the utility class if they don't want to pollute IntelliSense, and on the other hand they can include the Extensions namespace and use as an extension if they want. The neat thing is it also adheres to the Single Responsibility Principle. The XmlUtility is responsible for converting objects to XML, and the XmlExtensions is responsible for extending object's interface for ToXml().
