Expressive Code: Good Variable Names Instead Of Magic Values And Calculations

I like to remind myself of these little principles that I take for granted, now and then. It’s good habit to get back to basics and really understand why you hold principles so that you can judge whether or not they are appropriate in the circumstances you are in. Today’s little nugget of principle is to write expressive code and avoid magic values in your code.

For example, I was writing some graphics code for a specialized button in Compact Framework development. I wanted to draw an outline around my box but only the left side, top and right side – leaving the bottom blank. I started with this code:

   1: graphics.DrawLine(outlinePen, 0, 0, 0, ClientRectangle.Height);

   2: graphics.DrawLine(outlinePen, 0, 0, ClientRectangle.Width, 0);

   3: graphics.DrawLine(outlinePen, ClientRectangle.Width-1, 0, ClientRectangle.Width-1, ClientRectangle.Height);

(Note: ClientRectangle is provided by inheriting from Control in the compact framework. It’s the visible rectangle of the control.)

This code works perfectly and does what I want. In writing this code, I had to stop and think about the values that needed to go into each position as I was writing each one of these lines of code. It only took a few seconds to complete all three lines of code so it was not a big deal. However, I decided that I didn’t like this code because I found it very difficult to read. Every time I look at it (even though I wrote it) I have to remember which values are paired together to form which x/y coordinates, so that I can visualize what this will look like. Add in the object.property use and the simple math and there’s more work to read this code than I care for.

So, I fixed it. I removed the magic values (no hard coded integers) and I created variables that expressed what the math calculations represented.

   1: int left = 0;

   2: int right = ClientRectangle.Width - 1;

   3: int top = 0;

   4: int bottom = ClientRectangle.Height;

   5:  

   6: graphics.DrawLine(outlinePen, left, top, left, bottom);

   7: graphics.DrawLine(outlinePen, left, top, right, top);

   8: graphics.DrawLine(outlinePen, right, top, right, bottom);

Sure, I have 4 extra lines of code now. But look at the three lines of code that do the actual drawing compared to what was there previously. I don’t have to parse and calculate to understand where the line is being draw anymore. I can simply read “left-top to left bottom” and understand much faster “oh, this draws along the left border.”

Don’t just fulfill the technical requirements of the language syntax that you are writing. Write code that expresses your intent and an understanding of that intent that can be transferred to anyone else who reads the code.


Post Footer automatically generated by Add Post Footer Plugin for wordpress.

About Derick Bailey

Derick Bailey is an entrepreneur, problem solver (and creator? :P ), software developer, screecaster, writer, blogger, speaker and technology leader in central Texas (north of Austin). He runs SignalLeaf.com - the amazingly awesome podcast audio hosting service that everyone should be using, and WatchMeCode.net where he throws down the JavaScript gauntlets to get you up to speed. He has been a professional software developer since the late 90's, and has been writing code since the late 80's. Find me on twitter: @derickbailey, @mutedsolutions, @backbonejsclass Find me on the web: SignalLeaf, WatchMeCode, Kendo UI blog, MarionetteJS, My Github profile, On Google+.
This entry was posted in .NET, Analysis and Design, AntiPatterns, C#, Craftsmanship, Principles and Patterns. Bookmark the permalink. Follow any comments here with the RSS feed for this post.
  • http://simpleprogrammer.com John Sonmez

    Good example. I have written the same code before and been confused, because I didn’t do what you are suggesting here.

    Here is an even clearer version at the cost of some more code:

    private class LineSegment
    {
    public int StartX { get; set; }
    public int EndX { get; set; }
    public int StartY { get; set; }
    public int EndY { get; set; }
    }

    StartY, lineSegment.EndX, lineSegment.EndY);
    }

  • http://simpleprogrammer.com John Sonmez

    Whoops that was wierd, let me try one more time.

    private class LineSegment
    {
    public int StartX { get; set; }
    public int EndX { get; set; }
    public int StartY { get; set; }
    public int EndY { get; set; }
    }

    int left = 0;
    int right = ClientRectangle.Width – 1;
    int top = 0;
    int bottom = ClientRectangle.Height;

    IList lineSegments = new List {{StartX = left, StartY = top, EndY = left, EndY = bottom},
    {StartX = left, StartY = top, EndY = right, EndY = top},
    {StartX = right, StartY = top, EndY = right, EndY = bottom}};

    foreach(var lineSegment in lineSegments)
    {
    graphics.DrawLine(outlinePen, lineSegment.StartX, lineSegment.StartY, lineSegment.EndX, lineSegment.EndY);
    }

  • dave-ilsw

    Should bottom subtract one from Height like right subtracts one from Width?

  • PatrickM

    That’s one of the goals for the new syntax in C# 4.0:

    If you have a method:
    public void Do(string action, int time);

    You can optionally call it this way:
    Do(action: “Execute”, time: 5);

    And by the way, the parameter ordering does not have to be the same as the definition:
    Do(time: 5, action: “Execute”);

    Regards,

  • David Pendray

    Is 8 extra setup lines excessive? If not, I’d put forward:

    int left = 0;
    int right = ClientRectangle.Width – 1;
    int top = 0;
    int bottom = ClientRectangle.Height;

    var leftTop = new Point(left, top);
    var rightTop = new Point(right, top);
    var leftBottom = new Point(left, bottom);
    var rightBottom = new Point(right, bottom);

    graphics.DrawLines(outlinePen, new[] { leftBottom,
    leftTop,
    rightTop,
    rightBottom });

    With the final call reading: “leftBottom to leftTop to RightTop to RightBottom”.

  • http://simpleprogrammer.com John Sonmez

    @David

    I like that one.

  • http://www.lostechies.com/members/derick.bailey/default.aspx derick.bailey

    @dave-ilsw

    if i was drawing a line along the bottom of the button, then yes it would matter and it should have a -1 on the bottom variable. but in this case, i am only drawing on the left, top, and right borders so it doesn’t matter.

  • http://togaroga.com Andrew Hare

    This is really good advice as magic values really hurt the readability and maintainability of your code. I would argue that this is a hallmark of a declarative style of coding (being more expressive and ultimately more readable). I wrote a similar article myself (http://togaroga.com/2010/03/writing-better-code-its-imperative-that-you-are-declarative/) that focused more on method names than variable names but the concept is the same – declarative code is better for everyone!

  • Ufuk Kayserilioglu

    I am all for the more readable code; however, the two versions of the code do not produce the same output. In the first one, for the second DrawLine call you have a “ClientRectangle.Width”. In the more readable version that gets changed to “right”, which actually stands for “ClientRectangle.Width – 1″.

    Am I missing something?