Avoiding duplicated code with delegates…

Recently I looked at removing aliases when their owning items were deleted. I noted at the end of the post that the code ended up with some duplication in it. Duplication is generally a bad thing in code – copy/pastes of code tends to diverge over time and introduce bugs. So ideally we’d work out a way to get rid of the duplication, and reduce the set of methods our class needs.

Having been reading a bit on functional languages and F# in particular, I was thinking about how it can be done by passing code as parameters to more generic methods.

So looking back at the code from the other week, the pipeline class had the following methods:

public class DeleteItems : ItemOperation
{
    public void CheckAndDeleteAliases(ClientPipelineArgs args);

    private Database getDatabase(ClientPipelineArgs args);

    private void removeAliases(ListString items, Database db);
    private void removeAliases(Item item);

    private int countAliases(ListString items, Database db);
    private int countAliases(Item item);
}

The key thing to note here is the similarity between the two removeAliases() and the two countAliases() methods. And looking at the code, the two methods with the xxAliases(ListString items, Database db) signature seem very similar:

image

In fact the only differences are in text strings, the return type and the other function they call.

So how do we reduce the amount of code we need? Well, how about we try passing some of those differences in as parameters to a more generic function? For strings and normal types that’s easy – we do it all the time. But what about the other function that gets called to process the individual Items? Well in .Net we have the concept of Delegates to represent pointers to functions. Historically you had to declare these yourself, but more recently the arrival of Linq gave the .Net Framework a series of generic types for representing pointers to methods and functions: Action<T> and its variants represent a pointer to a void method. Func<T, Result> and its variants represent a pointer to a function that returns the type TResult. You can read more about the Func and Action types at MSDN.

So if we want to make our “do something” method into a parameter to some more generic code, we need to decide what it’s signature should be. In this case we have two possibilities: Action<Item> or Func<Item, int> depending on which version of the xxAliases() methods you look at from last week’s code. Since we can always ignore a return type if we want to, it seems that Func<Item, int> is the right choice here, as it lets us express both of our possible methods.

Based on that and a bit of thinking, we can replace the two xxAliases(ListString, Database) methods with one new method that looks like this:

private int processItems(ListString items, Database db, string taskContext, Func<Item, int> process)
{
    int count = 0;

    try
    {
        using (new TaskContext(taskContext))
        {
            using (new SecurityDisabler())
            {
                foreach (string item in items)
                {
                    Item itm = db.GetItem(item);
                    if (itm != null)
                    {
                        // Count this one
                        count += process(itm);

                        // And process any descendent items too
                        foreach(var descendantItm in itm.Axes.GetDescendants())
                        {
                            count += process(descendantItm);
                        }
                    }
                }
            }
        }
    }
    catch (Exception ex)
    {
        Log.Error("Error during: " + taskContext, ex, this);
        HttpUnhandledException ex2 = new HttpUnhandledException(ex.Message, ex);
        string htmlErrorMessage = ex2.GetHtmlErrorMessage();
        UrlString urlString = new UrlString("/sitecore/shell/controls/error.htm");
        Context.ClientPage.ClientResponse.ShowModalDialog(urlString.ToString(), htmlErrorMessage);
    }

    return count;
}

It does exactly the same thing, except that it takes the processing function for each Item as a parameter, and it also takes the name of the task for logging / error purposes as a parameter. And with that we can remove the two methods with the xxAliases(ListString, Database) signature.

Making use of this is easy, since C# will implicitly type cast the name of a function into the appropriate Func<> type at compile time, so for counting aliases we can just write:

// Count the aliases for any items we have as parameters
int aliases = processItems(items, db, "DeleteItems pipeline - count aliases", countAliases);

and that will cause our countAliases(Item) method to be called for each item the processItems() code finds. But doing the same thing for the removeAliases() method would give a compiler error. That returns void, meaning it can’t be typecast to a Func<> – so we have to cheat a little here and make removeAliases(Item item) return an integer instead of void. You can count the number of Alias items that actually get removed, or you can just return 0 – it doesn’t matter to the rest of the code.

It’s worth mentioning here that you don’t actually need to declare a method to call our processItems() function. Another of the nice features you get with this approach is that you can write methods and functions inline using Lambda expressions. So you can write:

processItems(listOfItems, database, "Example anonymous method", itm => {
    // do something
    return 0;
});

And the compiler magic behind “=>” works out that what you’ve declared and creates a method with a computer generated name and the correct signature without you having to type it all out.

So with those changes made, our class now contains the following methods:

public class DeleteItems : ItemOperation
{
    public void CheckAndDeleteAliases(ClientPipelineArgs args);

    private Database getDatabase(ClientPipelineArgs args);

    private int processItems(ListString items, Database db, string taskContext, Func<Item,int> process);

    private int removeAliases(Item item);
    private int countAliases(Item item);
}

And we’ve successfully removed the duplication. Job’s a good ‘un.

Advertisements

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s