Skip to main content

Copy Right

This has nothing to do with intellectual property. Instead it is really about using Copy and Paste intelligently.

Computers are really good at repeating things over and over again, whereas humans are not. So when it comes to repeating similar tasks, we should instruct the computer to repeat the task instead of us repeatedly instructing the computer to do the task.

Classic Copy'n'Paste

Often I come across code similar to the following, which updates a Form's user interface to put error icons on text box entries where the value is empty.

private ErrorTagHandler _errorTagHandler;

private void UpdateInterface()
{
    textBoxUserName.Text = _userName;

    if (string.IsNullOrEmpy(_userName))
    {
        ErrorTag error = new ErrorTag("User Name required");
        _errorTagHandler.Associate(textBoxUserName, error);
    }
    else
        _errorTagHandler.ClearAssociation(textBoxUserName);

    textBoxPassWord.Text = _passWord;

    if (string.IsNullOrEmpy(_passWord))
    {
        ErrorTag error = new ErrorTag("Password required");
        _errorTagHandler.Associate(textBoxPassWord, error);
    }
    else
        _errorTagHandler.ClearAssociation(textBoxUserName);

    textBoxEmail.Text = _email;

    if (string.IsNullOrEmpy(_email))
    {
        ErrorTag error = new ErrorTag("Email required");
        _errorTagHandler.Associate(textBoxEmail, error);
    }
    else
        _errorTagHandler.ClearAssociation(textBoxEmail);
}

As you can see we are doing essentially the same thing over and over again. The original author probably wrote the code for textBoxUserName and then copied it for textBoxPassWord and textBoxEmail , changing the text literal "... required" and the source variable (to _passWord and _email ).

You might ask yourself: “What could possibly be wrong with this?”

Overlooking one part

If you are using copy and paste you have to make sure that you update all the relevant parts of the snippet that you have just inserted.

Look at the code again and you will see that there is a bug. In the case where the password is not blank, the call to clear the errorTag is updating textBoxUserName when it should have been updating textBoxPassword . That's because our programmer was in a hurry and missed that one.

Will it compile? Yes, of course it will. Will you catch this error during testing? Maybe, maybe not.

Change in approach

Let's imagine that you decided that the ErrorTagHandler class was not the right way of indicating the error state on a control. So now you are going to use the SuperSmartHighlighter class instead. The problem is that the SuperSmartHighlighter , of course, does have a different interface to the ErrorTagHandler .

This means finding all occurrence of calls to ErrorTag and ErrorTagHandler methods and changing them. While this might seem trivial enough with the above example, most projects would have similar code in lots of places. So that means that there is a lot of code to be changed.

Move Repeating code into a function

Consider this alternative:

private ErrorTagHandler _errorTagHandler;

private void SetErrorState(TextBox textBox, string value, string message)
{
    textBox.Text = value;

    if (string.IsNullOrEmpty(value))
    {
        ErrorTag error = new ErrorTag(message);
        _errorTagHandler.Associate(textBox, error);
    }
    else
        _errorTagHandler.ClearAssociation(textBox);
}

private void UpdateInterface()
{
    SetErrorState(textBoxUserName,  _userName,  "User Name required");
    SetErrorState(textBoxPassWord,  _passWord,  "Password required");
    SetErrorState(textBoxEmail,     _email,     "Email required");
}

Not only is this code now significantly shorter, but it is also a lot more readable, maintainable and less prone to errors.

Of course, we can still improve on this. We could extend the ErrorTagHandler class thus:

// if we have access to the source

    void SetErrorState(TextBox textBox, string value, string message)
    {
        textBox.Text = value;

        if (string.IsNullOrEmpty(value))
        {
            ErrorTag error = new ErrorTag(message);
            Associate(textBox, error);
        }
        else
            ClearAssociation(textBox);
    }

// if we don't have acesse to the source
public static class ErrorTagHandlerExtension
{
    public static void SetErrorState(this ErrorTagHandler errorTagHandler, TextBox textBox, string value, string message)
    {
        textBox.Text = value;

        if (string.IsNullOrEmpty(value))
        {
            ErrorTag error = new ErrorTag(message);
            errorTagHandler.Associate(textBox, error);
        }
        else
            errorTagHandler.ClearAssociation(textBox);
    }
}

and change the calling code to become:

private ErrorTagHandler _errorTagHandler;

private void UpdateInterface()
{
    _errorTagHandler.SetErrorState(textBoxUserName,  _userName,  "User Name required");
    _errorTagHandler.SetErrorState(textBoxPassWord,  _passWord,  "Password required");
    _errorTagHandler.SetErrorState(textBoxEmail,     _email,     "Email required");
}

shorter still and allows us to leverage the same bit of code in other forms with minimum of fuss.

Generic Parsing

If you need to do parsing of configuration files, you probably have lots of code that tries to convert some textual values into enumeration values similar to this:

enum TextColor
{
    Unspecified
    Black,
    White,
    Red,
    Purple,
    Pink,
    Orange,
    Green,
    Lime
};

TextColor _headingColor;
TextColor _paragraphColor;
TextColor _listsColor;
TextColor _footerColor;
TextColor _headerColor;

void SomeMethod(ConfigValues values)
{
    if (string.IsNullOrEmpty(values["heading"]))
        _headingColor = TextColor.Unspecified;
    else
        _headingColor = (TextColor)Enum.Parse(typeof(TextColor), values["heading"]);
         
    if (string.IsNullOrEmpty(values["paragraph"]))
        _paragraphColor = TextColor.Unspecified;
    else
        _paragraphColor = (TextColor)Enum.Parse(typeof(TextColor), values["paragraph"]);

    if (string.IsNullOrEmpty(values["lists"]))
        _listsColor = TextColor.Black;
    else
        _listsColor = (TextColor)Enum.Parse(typeof(TextColor), values["lists"]);

    if (string.IsNullOrEmpty(values["footer"]))
        _footerColor = TextColor.Black;
    else
        _footerColor = (TextColor)Enum.Parse(typeof(TextColor), values["footer"]);

    if (string.IsNullOrEmpty(values["header"]))
        _headerColor = TextColor.Black;
    else
        _headerColor = (TextColor)Enum.Parse(typeof(TextColor), values["header"]);

If you have to do lots of parsing of type TextColor , you can probably wrap this up in a function like this:
TextColor ParseTextColor(string str, TextColor defaultValue)
{
    if (string.IsNullOrEmpty(str))
        return defaultValue;

    return (TextColor)Enum.Parse(typeof(TextColor), str, true);
}

and then your calling code becomes a simple

    _headingColor = ParseTextColor("heading", TextColor.Unspecified);
    _paragraphColor = ParseTextColor("paragraph", TextColor.Unspecified);
    _listsColor = ParseTextColor("lists", TextColor.Unspecified);
    _footerColor = ParseTextColor("footer", TextColor.Unspecified);
    _headerColor = ParseTextColor("header", TextColor.Unspecified);

but you will probably find yourself doing lots of parsing of different enum types. Having to write a function for each of these becomes tedious and we are again in the realm of doing copy and paste.

Instead we can do this task with a simple generic extension class: Enum.cs

While this might look like a lot of code to do a simple tasks, consider what it enables us to do:

[DefaultEnum(Unspecified)]
enum TextColor
{
    Unspecified
    Black,
    White,
    Red,
    Purple,
    Pink,
    Orange,
    Green,
    Lime
};

TextColor _headingColor;
TextColor _paragraphColor;
TextColor _listsColor;
TextColor _footerColor;
TextColor _headerColor;

void SomeMethod(ConfigValues values)
{
    _headingColor = Enum<TextColor>.Parse(values["heading"], true);
    _paragraphColor = Enum<TextColor>.Parse(values["paragraph"], true);
    _listsColor = Enum<TextColor>.Parse(values["list"], true, TextColor.Black);
    _footerColor = Enum<TextColor>.Parse(values["footer"], true, TextColor.Black);
    _headerColor = Enum<TextColor>.Parse(values["header"], true, TextColor.Black);
}

If the input string does not match any of the enum values, _headingColor and _paragraphColor will default to TextColor.Unspecified while for the others we provide the value TextColor.Black .

Using this generic approach allows us to use similar code to this with any enum that we might encounter - truly re-usable code.