Extension Methods - The devious evils lurking in your code

If like me you do a lot of programming you have probably run across so-called 'bad code smells'. These are things you see in code that make you a bit queasy when you see them because with enough experience you start thinking 'I don't think we should do it that way' and think of myriad of reasons not to do it that way.

This feeling of unease is built by seeing how bad it turned out the last time someone tried it, what it's really doing, and what kind of programmers left the code that way (we all do it sometime).

Extension methods, what are those ?

Many programming languages have them but basically it's a way to extend the functionality of a class without modifying the class. Take this code for example:

// BankAccount.cs
public class BankAccount
{
        private double _balance = 0;
        public double Balance => _balance;
        public void Deposit(double amount)
        {
                 _balance += amount;
        }

        public void Withdraw(double amount) => Deposit(amount * -1)
}

//...

// BankAccountExtension.cs
public static class BankAcountExtension
{
      public static void WithdrawIfAvailable(this BankAccount bankAccount, double amount)
      {
            if(amount < 0) 
                throw new ArgumentException(The amount to withdraw must be positive!",  "amount");

            if(bankAccount.Balance >= amount) 
                bankAccount.Withdraw(amount); 
      }

      // probably lots of other code in this 'class', though it isn't really a class.
}

Great this allows me to call this method as if it existed on my BankAccount Class like so:

public static void Main(string[] argv)
{
 //  ...
 var bobsBankAccount = new BankAccount();

 bobsBankAccount.Deposit(120.34);

 // ...

 bobsBankAccount.WithdrawIfAvailable(120); // it works fine but why ?

}

Ok what's wrong with that ?

This is where it helps to really understand the basics of C# and object-oriented programming.

Static methods don't exist on instances by design

A static method or property is designed in the language to be a concept that exists outside a class and it's really only accessible to calling code in two ways:

1. From an instance of the class the static method is defined with (note in this case there's no instance of the class as it is a static class)
2. Directly from the name of the class on which the static is defined, like this:

BankAcountExtension.WithdrawIfAvailable(bobsBankAccount, 120);

Extension methods are by design independent of actually having an instance of a class.

This is the point I was building to, I can now write these two semantically equivalent calls:

// a call from the named static class class directly with no instance necessary
BankAcountExtension.WithdrawIfAvailable(bobsBankAccount, 120);

// a call on the instance style
bobsBankAccount.WithdrawIfAvailable(120);

This to me is a problem for several reasons.

1. This confuses programmers looking at these calls
2. It allows for two ways to write precisely the same code that will be compiled exactly the same
3. It is an illusion of object-oriented programming
4. It spirals out of control when used wrong
5. people using it for readability have now confused their readers
6. different people on the same project tend to prefer one of the ways to write over the other leading to inconsistent code (without code reviews and other protections)
7. people tend to use them for the wrong reasons
8. It flattens your 'object-oriented design' by fooling you into thinking you have object-oriented code when you really have procedural code.
9. They are a pain in the !@#$ to test especially when they start depending on each other (without something like TypeMock).

Don't get me wrong extension methods have their place but many programmers use them wrong and overuse them leading to spaghetti code that pretends to be object-oriented through some Syntactic Sugar. When used properly to simply add functionality to a class that you couldn't get otherwise because the source is closed to you and you really want things to act as if they are object-oriented in code.

Anyone else have any thoughts on this ?