Single Letter Variable Names Still Considered Harmful

Monday, November 17, 2008

There is a lot of humor in the Bad Variable Names entry on the c2 wiki. I like this confession from Alex:

The worst of which was my counter variable names. I now use i, j, k, and so on for local counts and things like activeRowCount for the more descriptive names. Before, in the early years mind you, it shames me to say, I would name my counters things like Dracula, Chocula*, MonteChristo. They are all counts after all. I apologize for my intial variable naming conventions and shall go beat my face now as punishment.

(*If you are not familiar with Count Chocula, he’s the mascot of a chocolate flavored breakfast cereal of the same name. The cereal is popular in America, as is any corn based cereal that mixes chocolate and sugar.)

One letter variable names are considered bad, unless you need a loop counter (i), a spatial coordinate ( x,y,z) …

… or you’re writing a lambda expression:

var query = employees.Where(e => e.Name == "Tom");

This isn’t just C# – other languages also encourage short variable names in nested function definitions. The short names have been bothering me recently, particularly when the chained operators begin to work on different types.

var query = employees.GroupBy(e => e.Name)
                     .Where(g => g.Count() > 2)
                     .Select(g => g.Key);

LINQ joins are particularly unreadable. There is a piece of code I wrote in a presenter class many months ago, and every time I see the code I wince.

var controlsToEnable =
        results.SelectMany(r => r.RequiredFields)                
               .Join(mapper.Entries,
                       ps => ps.PropertyName,
                       e => e.Left.PropertyName,
                       (ps, e) => e.Right)
               .Select(ps => ps.GetValue<IAnswerControl>(_view));

I’ve been thinking that one way to improve the readability is to use a custom Join operator. The types used in the query above are PropertySpecifier<T> and MapEntry<TLeft, TRight>. The idea is to join property specifiers against the left hand side of a mapping entry to retrieve the right hand side of the mapping entry (which is another property specifier). The extension method has a lot of generics noise.

public staticIEnumerable<PropertySpecifier<TRight>> Join<TLeft, TRight>(
      thisIEnumerable<PropertySpecifier<TLeft>> specifiers,
    IEnumerable<MapEntry<TLeft, TRight>> entries)
{
    return specifiers.Join(entries,
propertySpecifer => propertySpecifer.PropertyName,
mapEntry => mapEntry.Left.PropertyName,
(propertySpecifier, mapEntry) => mapEntry.Right);
}

At least the custom Join cleans up the presenter class query, which I also think looks better with more descriptive variable names.

var controlsToEnable =
        results.SelectMany(result => result.RequiredFields)
               .Join(mapper.Entries)
               .Select(property => property.GetValue<IAnswerControl>(_view));

For the time being I'm going to avoid one letter variable names in all but the simplest lambda expressions.


Comments
friism Monday, November 17, 2008
Underscore (_) is a valid variable name in C# and works great for simple lambdas. I haven't tested for scaling with multiple underscores though (__, ___, ..nah).
Mikael Lundin Monday, November 17, 2008
EventArgs e
scott Monday, November 17, 2008
@friism - hmm, not sure about underscores.

@mikael - touche!
Comments are now closed.
by K. Scott Allen K.Scott Allen
My Pluralsight Courses
The Podcast!