6

The syntactic sugar provided by extension methods can be addictive.

Take for example

public void TagNode(SiteMapNode childNode, string url, string title)
{
  //do stuff
}

vs

public static void Tag(this SiteMapNode source, string url, string title)
{
  //do stuff
}

and the usage:

TagNode(myNode, myUrl, myTitle);

vs

myNode.Tag(myUrl, myTitle);

You can see and feel the sugar, and after the extension is created there is less typing.

But I'm wondering if it can go too far and how to judge what's too far and is this a known anti-pattern?

If it's an anti-pattern but it doesn't have a name, I'd call it:

The hyper-extension anti-pattern.

  • 2
    Extension methods have one limitation: they don't have access to the innards of the class. They really are syntactic sugar; real methods would have access to the class' innards. – Robert Harvey Aug 30 '16 at 0:06
  • In that case (granular) functional programming is also an anti-pattern. – Den Aug 30 '16 at 12:12
  • Extension methods also have a risk: name collisions – S.D. Sep 25 '18 at 13:45
6

Extension methods are very useful for making lightweight interfaces. The prototypical example of IEnumerable has only 1 member. All the functionality of the extension methods apply to your class ExoticContainer<T> : IEnumerable<T> without extra effort.

Extension methods are less appropriate for classes that you control, where you should just add them as normal methods.

In the middle, where you have a class from somewhere else, it becomes a style choice between the TagNode(myNode, myUrl, myTitle); and myNode.Tag(myUrl, myTitle); syntaxes. Both styles are equally valid, but sticking with one or the other is preferable to a mixture.

  • I think you are missing the core question. ie why extension methods instead of a wrapper object, service, injection into another object etc etc – Ewan Aug 30 '16 at 15:00
  • @Ewan Not really. If it is expressible as static void Tag(SiteMapNode source, string url, string title) then adding a this and placing it in an appropriate static class is a style choice. – Caleth Aug 30 '16 at 15:09
  • not entirely. By using statics you make your DI and testing harder and what if you have some methods which can be static and some which require state? your extension methods have to be on a static class, so how do you group by responsiblity? – Ewan Aug 30 '16 at 15:20
  • That doesn't follow. Because extension methods are considered last in override resolution, you can write a class MockSiteMapNode : (I?)SiteMapNode {public void Tag(string url, string title)} for testing. If you have other actions that require state, then they need to have access to that state, e.g. as members of a class containing such state. You can still assign the extension method to an Action<SiteMapNode, string, string> to pass it around if you like – Caleth Aug 30 '16 at 15:36
  • not if the extension method use is buried in another class. you are unable to inject a mock version. – Ewan Aug 30 '16 at 15:42
3
  1. Taking a private function and making it public is not usually ideal.
  2. If you're using so little of a class that you can make many extensions methods for it, why is it a class?
  3. If you're going for a more functional approach, then making stuff look object oriented is just silly.

All that said, this isn't an anti-pattern. Anti-patterns are always harmful. The LINQ extensions for IEnumerable are a clear counter argument. Overuse of extension methods is a smell at worst.

  • @DocBrown - 1 in the OPs example, the visibility changes. Not limited to EMs. Otherwise, I'm trying to figure out why someone would make bunches of extensions. – Telastyn Aug 30 '16 at 11:43
  • @doc a common thing I see is using extensions to implement a fluent style interface – Ewan Aug 30 '16 at 14:14
0

I would go further, I don't like extension methods at all.

If I wrote:

Static class MyHelper {
    Static public int CountEmployees(MyCompany company) {...

Rather than

Public class MyCompany {
     Public int EmployeeCount() { ..

I don't think anyone would say it was a good pattern

It's only the syntactic sugar provided by the compiler making these helpers look like methods which encourages you to create them at all.

If you have a 3rd party class you want to extend (such as IEnumerable) there are other ways to do it. You have to really really want the 'looks like a method on another class' feature to go with extensions in my view.

I guess you can see why they did it with IEnumerable. Adding the features to the interface would have forced everyone to update all their collections with extra stuff, adding new ILinqEnumerable versions of everything would mean having both versions hanging around and static helpers avoid the creation of new objects for every clause in your statement

  • 2
    the static method i know can only do things that the public interface of Company allows, the mathod can mess around with internal stuff I'd have to read the source to know that, so no I don't think making everything a member function is a good idea – jk. Aug 30 '16 at 9:06
  • edited for clarity. ie my example is where you are writing both classes – Ewan Aug 30 '16 at 11:57
  • Your EmployeeCount method should actually be a property or if it should stay a method then it's name should be the same as the extensions. – t3chb0t Sep 13 '16 at 12:36

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.