Using extension methods to hide infrastructure code

Extension methods have been around for a while in C# by now, and sometimes you see questions regarding the real use of this approach. That can be debated, and this text is not aiming to cover the subject in whole, but instead show on one case where you may find them usable.

Recently I was working on a piece of code that needed a lot of synchronization to control access to a shared resource (that code will be covered in a coming blog post). I used the ReaderWriterLockSlim for this, which sometimes produced code blocks like the following (this code sample has, as usual, been created just for demo purposes):

private static int GetFromQueueWithoutExtensions()
{
    int result = -1;
    _lock.EnterUpgradeableReadLock();
    try
    {
        if (_sharedResource.Count > 0)
        {
            _lock.EnterWriteLock();
            try
            {
                result = _sharedResource.Dequeue();
            }
            finally
            {
                _lock.ExitWriteLock();
            }
        }
    }
    finally
    {
        _lock.ExitUpgradeableReadLock();
    }

    return result;
}

_sharedResource is a static Queue instance, declared elsewhere in the class. The above method contains 12 lines of code (disregarding blank lines, and lines consisting of only a curly brace). In these 12 lines of code, only 4 are directly related to the task of the function, the remaining 8 lines being what we could call infrastructure; various control mechanisms to make sure that appropriate locks are acquired and released. This can make the code harder to read, since you will need to filter out the (minority of) lines that carry out the actual functionality of the method.

If we investigate the code structure a bit closer, we can see that the pattern around running code while holding a certain lock looks like this (pseudo code):

acquire the lock
try
{
   execute the task while holding the lock
}
finally
{
   release the lock
}

This is a repeating pattern, so now I see two reasons to refactor the original code sample shown above; try to move infrastructure out of the method, and stick to the DRY (Don't Repeat Yourself) principle.

I will start by trying to encapsulate the "run code under lock" pattern into a separate method:

private static void RunWithUpgradeableReadLock(ReaderWriterLockSlim readerWriterLock, Action action)
{
    readerWriterLock.EnterUpgradeableReadLock();
    try
    {
        action();
    }
    finally
    {
        readerWriterLock.ExitUpgradeableReadLock();
    }
}

The above code needs to be repeated for each lock method (so methods encapsulating EnterReadLock and EnterWriteLock need to be added). Now, the calling code can be shortened down quite considerably:

private static int GetFromQueueWithEncapsulation()
{
    int result = -1;

    RunWithUpgradeableReadLock(_lock, () =>
    {
        if (_sharedResource.Count > 0)
        {
            RunWithWriteLock(_lock, () =>
            {
                result = _sharedResource.Dequeue();
            });
        }
    });

    return result;
}

Now, by making a couple of small modifications to the encapsulating method, we can make it into an extension method. First we need to move it into a class that is declared as public static. Second we need to add the keyword this in front of the ReaderWriterLockSlim parameter:

public static class ReaderWriterLockSlimExtensions
{
    public static void RunWithUpgradeableReadLock(this ReaderWriterLockSlim readerWriterLock, Action action)
    {
        readerWriterLock.EnterUpgradeableReadLock();
        try
        {
            action();
        }
        finally
        {
            readerWriterLock.ExitUpgradeableReadLock();
        }

    }
}

By doing this you add one feature above all (in my opinion): discoverability. The method RunWithUpgradeableReadLock method will show up in IntelliSense when using a ReaderWriterLockSlim:

extensionmethods.intellisense

In order to have your code call the extension method rather than the encapsulated method shown earlier, you write code that invokes the method on the lock object rather than passing the lock object as an argument:

private static int GetFromQueueWithExtensions()
{
    int result = -1;

    _lock.RunWithUpgradeableReadLock(() =>
    {
        if (_sharedResource.Count > 0)
        {
            _lock.RunWithWriteLock(() =>
            {
                result = _sharedResource.Dequeue();
            });
        }
    });

    return result;
}

This code block contains two extra lines of code (plus scoping curly braces) for the locking mechanisms. In my opinion this is making the code much more to the point, making it easier to read and understand what it does.