Refactoring copy/paste code with the use of delegates

Introduction

In this article, I will propose a tactical solution for Copy/Paste patterns in C#, where methods share most of the code except for a line or two.

Background

For most of my professional life, I had to deal with other people's code, and that made me develop some very useful code maintenance skills. But the ability to read other people's code doesn't lessen the frustration of having to deal with badly written code. It's not only frustrating because one has to fix and extend it, but trying to figure out where it will go wrong and also be responsible for dealing with the consequences of the bad thinking behind it.

Making things bad to worst, some managers hinder any refactoring effort, deriding it as gold platting and a waste of time and resources, which should have been put on developing new features and maintaining the project on track with the current schedule.

The fact is, code rotting is a major source of headaches, a source of endless bugs and schedule slips. Not dealing with the issue endangers the project and puts the team in needless stress. But there are also other concerns, some members of the team lack technical skills and the will to learn new ones. So, they are comfortable with the rotting code, because they understand it better and any change will be stressful for them.

In this kind of scenario, refactoring has to be done by stealth, which means the architecture of the application will stay, but some troublesome code files will be refactored. Maintaining public signatures and overall silly functions that do too much to be changed right away without getting angry noises.

Let's get to the code, shall we?

We start with a set of methods that present the following pattern:

public XmlDocument ThisMethodDoesSomethingImportant(string xmlString)
{
    XmlDocument XmlOut = new XmlDocument();
    string Error = "";
    
    this.DoesSomething(out Error);
    
    if (Error != "")
    {
        // Do Some Trace code here...
        
        XmlOut.LoadXml("<Error>" + Error + "</Error>");
        
        return XmlOut;
    }
    
    SomeBusinessClass business = new SomeBusinessClass(this.SomeInitialVar, 
                                 this.SomeOtherVarThatIsInitiadedOnDoesSomething);
    
    try
    {
        // this is the only that changes
        XmlOut = business.SomeMethod(xmlString);
    }
    catch (Exception ex)
    {
        // Do Some Trace code here...
        
        XmlOut.LoadXml("<Error>" + Error + "</Error>");
        
        return XmlOut;
    }
    
    return XmlOut;
}

This class has a bunch of methods, all or most of them are just a copy; the challenge is to delete most of the copied code and keep its form while getting something simpler to maintain.

The solution is to use delegates in .NET 2.0, and Lambda functions in 3.5 and over, but first, we need to separate the body of the method from the public method call.

In .NET 2.0:

private delegate XmlDocument DelegatedFunction(SomeBusinessClass business);

// Now this is the repeated pattern
public XmlDocument ThisMethodDoesSomethingImportant(string xmlString)
{
    DelegatedFunction function = delegate(SomeBusinessClass business)
    {
        // changed method here
        return business.SomeMethod(xmlString);
    };

    return this.DoesSomethingBody(function);
}

private XmlDocument DoesSomethingBody(DelegatedFunction function)
{
    XmlDocument XmlOut = new XmlDocument();
    string Error = "";
    
    this.DoesSomething(out Error);
    
    if (Error != "")
    {
        // Do Some Trace code here...
        
        XmlOut.LoadXml("<Error>" + Error + "</Error>");
        
        return XmlOut;
    }
    
    SomeBusinessClass business = new SomeBusinessClass(this.SomeInitialVar, 
                                 this.SomeOtherVarThatIsInitiadedOnDoesSomething);
    
    try
    {
        // here is the change
        return function(business);
    }
    catch (Exception ex)
    {
        // Do Some Trace code here...
        
        XmlOut.LoadXml("<Error>" + Error + "</Error>");
        
        return XmlOut;
    }
}

In .NET 3.5 and above:

// Now this is the repeated pattern
public XmlDocument ThisMethodDoesSomethingImportant(string xmlString)
{
    Func<SomeBusinessClass,XmlDocument> function = 
           (SomeBusinessClass business) => business.SomeMethod(xmlString);
    
    return this.DoesSomethingBody(function);
}

private XmlDocument DoesSomethingBody(Func<SomeBusinessClass,XmlDocument> function)
{
    XmlDocument XmlOut = new XmlDocument();
    string Error = "";
    
    this.DoesSomething(out Error);
    
    if (Error != "")
    {
        // Do Some Trace code here...
        
        XmlOut.LoadXml("<Error>" + Error + "</Error>");
        
        return XmlOut;
    }
    
    SomeBusinessClass business = new SomeBusinessClass(this.SomeInitialVar, 
                                     this.SomeOtherVarThatIsInitiadedOnDoesSomething);
    
    try
    {
        // here is the change
        return function(business);
    }
    catch (Exception ex)
    {
        // Do Some Trace code here...
        
        XmlOut.LoadXml("<Error>" + Error + "</Error>");
        
        return XmlOut;
    }
}

What was done in these examples was to separate the method body, that has most of the shared code, from the initial version and send a delegate or an anonymous function to handle the variant section of the code. The public method is now only a façade that creates the delegate and sends it to the method body.

If it is the case that there are more input variables on the method signature, this can also be easily extended.

public XmlDocument ThisMethodDoesSomethingImportant2(string xmlString, 
                                                     string par1, int par2)
{
    Func<SomeBusinessClass,XmlDocument> function = 
      (SomeBusinessClass business) => business.SomeMethodExtended(xmlString, par1, par2);
    
    return this.DoesSomethingBody(function);
}

public XmlDocument ThisMethodDoesSomethingImportant3(string xmlString, 
                                                     string par1, string par2)
{
    Func<SomeBusinessClass,XmlDocument> function = delegate(SomeBusinessClass business)
    {
        business.SomeProperty = par2;
        return business.SomeMethodEx(xmlString, par1);
    };
    return this.DoesSomethingBody(function);
}

Points of interest

This refactoring still leaves a lot of ugliness behind, but gives you time to think about a better solution for refactoring the whole class and adjacent ones also. Hopefully, this can help you on your work without getting angry noises from co-workers and managers alike.

History

• First submission - 03-11-2010.