Robert Westerlund

A developer's blog on code, technology and tools in the Web, .NET and other development areas.

NAVIGATION - SEARCH

"GetMemberName" or "getting rid of (some) magic strings"

Earlier today I had another discussion() with a co-worker of mine. This discussion related to magic strings and potential ways to get rid of them. I have previously mentioned that I dislike magic strings, but I haven't mentioned why. Nor have I mentioned in which circumstances which I dislike seeing them.

When do I use magic strings

There are cases where magic strings are pretty useful. An example of this is in communicating between different applications, where strings are more descriptive than using, for example, an integer. Thus, if I send a message from application A to application B telling it to do “<action>print</action>” on something, it is a lot more descriptive and easy to debug than if it would be “<action>3</action>”. The same goes for the code listening to this message.

switch(action)
{
    case “print”:
        //Do the printing
        break;
}

it is a lot better than

switch(action)
{
    case 3:
        //Do the printing
        break;
}

However, I often try to limit the risk of errors, when using magic strings, for example by creating an XSD forcing an enumeration constraint (only allowing the content of the action element to be one of a limited set of values). This also has the benefit of making the message easier to use. (Whether it could still be called a magic string after enforcing an enumeration constraint on the value is a question of definitions, so in general I try to avoid the pitfalls of magic strings.)

When do I try to avoid magic strings

On the other hand, there are a lot of cases where I believe that magic strings are nothing but evil (although in some cases they may be necessary evil). One example of this is the INotifyPropertyChanged interface. Another such example is throwing ArgumentNullExceptions.

public class MyClass:INotifyPropertyChanged
{
    public MyClass(IRepository repository)    
    {
        if (repository == null)
        throw new ArgumentNullException(“repository”);
    }

    private string _name;
    public string Name
    {
        get
        {
            return _name;
        }
        set
        {
            _name = value;
            OnNotifyPropertyChanged(“Name”);
        }
    }

    public event PropertyChangedEventHandler PropertyChanged;
    protected void OnPropertyChanged(string propertyName)
    {
        if (PropertyChanged != null)
            PropertyChanged(this, new PropertyChangedEventArgs(propertyName);
    }
}

What happens when we change the IRepository to be a IDataRepository and therefore also renames the constructor parameter to dataRepository? What happens when we rename the Name property to Title? Well, in most cases we will remember to change the string as well, but every now and then we will forget to change the string. Also, sometimes we will misspell the property name, e.g. writing -“Neme” instead of “Name”. Of course, I could unit test all the property setters to make sure that the NotifyPropertyChanged is fired with the correct propertyName. While this might be a good (or bad) idea in and of itself, we still would have to remember to change the contents of the string in the unit test when we rename the property.

Expressions to the rescue

With C# 3.0 we got lambda expressions, which I am very fond of. Our nice lambda expressions are a lot better as a syntax for writing anonymous methods than what anonymous delegates ever were. The sometimes well-known secret here, however, is that the same lambda expression code could mean two completely different things, depending on the context. Given the following code:

DoStuff<string>(stringValue => Console.WriteLine(stringValue));

We would pass completely different things to our DoStuff-method depending on how that method is defined (Which could be compared to sending a constant integer 1 as a parameter to method. If the method takes an integer, the parameter would be compiled as an integer but if the parameter is defined as a double, the compiler would help us and change the 1 to a double with the value 1.0d). If the parameter is defined as an Action<string>, then we would receive a precompiled Action which we could use. If, on the other hand, the parameter is defined as Expression<Action<string>> then we would instead get an expression tree. Using this expression tree, we could compile and execute it if we want to, but we also have the possibility to examine the expression tree (which is very useful, for example when creating an IQueryable or IQbservable).

The GetMemberName-method

What I would want in the above examples is that if I would rename the property or parameter using the refactoring tools in Visual Studio, the string would automatically be updated as well. And if I, for some reason, would have misspelled or forgot to update the string, I would like a compiler error. The GetMemberName-method will help us achieve this goal.

public static class ObjectExtensions
{
    public static string GetMemberName<T, K>(this T obj, Expression<Func<T, K>> expression)
    {
        if (expression == null)
            obj.GetMemberName(_ => expression);
        MemberExpression memberExpression = expression.Body as MemberExpression;
        if (memberExpression == null)
            throw new ArgumentException("Expression of type " 
                + expression.Body.Type.ToString() 
                + " is not supported by the GetMemberName method."
                + " Only MemberExpressions are currently supported.", obj.GetMemberName(_ => expression));
        return memberExpression.Member.Name;
    }
}

We could then change our code above to something similar to the following:

public class MyClass:INotifyPropertyChanged
{
    public MyClass(IRepository repository)    
    {
        if (repository == null)
            throw new ArgumentNullException(this.GetMemberName(_ => repository));
    }

    private string _name;
    public string Name
    {
        get
        {
            return _name;
        }
        set
        {
            _name = value;
            OnNotifyPropertyChanged(this.GetMemberName(_ => Name));
        }
    }

    public event PropertyChangedEventHandler PropertyChanged;
    protected void OnPropertyChanged(string propertyName)
    {
        if (PropertyChanged != null)
            PropertyChanged(this, new PropertyChangedEventArgs(propertyName);
    }
}

Tiny drawbacks

Of course, for some unfortunate reason, there are no silver bullets which solve every problem, even though this one is at least silver coated. The drawback of using the GetMemberName instead of using a constant string is performance. I have performed very naive performance tests on the GetMemberName-method and it has shown that the method call took up to 30 milliseconds to perform on my laptop (which isn’t a very powerful laptop). In almost all cases I believe these milliseconds are worth getting rid of the strings, but if I would set the property 100 times per second this could be a problem (on the other hand, if I update it that frequently, perhaps it isn't a good fit for INotifyPropertyChanged). Therefore, I try to use this approach as often as I can, instead of using unnecessary magic string, and only if it would become a problem, I would consider writing the name as a string.

Unit Testing

It could be added, of course, that if you have unit tests which cover the PropertyChanged events when setting parameters this could be a non-issue, as long as you start a property rename with renaming in the tests and there renaming the string. Perhaps the GetMemberName method could be good to use in the unit test and to not use in the application code?

Add comment