Easy To Read != Easy To Understand

I ran into this code a while back:

   1: public abstract class ScanPrefixSpecification : IScanSpecification

   2: {

   3:     public abstract IEnumerable<string> BarcodePrefixesFilter { get; }

   4:     

   5:     public bool IsSatisfiedBy(string item)

   6:     {

   7:         return BarcodePrefixesFilter.Where(item.StartsWith).Any();

   8:     }

   9:  

  10:     public string GetScanFrom(string scanString)

  11:     {

  12:         foreach (var prefixFilter in BarcodePrefixesFilter.Where(scanString.StartsWith))

  13:         {

  14:             return scanString.Substring(prefixFilter.Length);

  15:         }

  16:         return "";

  17:     }

  18: }

There isn’t anything wrong with this code from a technical standpoint. It does exactly what it needs to do and it provides a specific point of functionality in our system. By reading the code and by reading the tests around this code, I can see what this class does:

  • it’s a basic specification pattern where the scanned item satisfies the specification when the “item” starts with any of the strings found in the BarcodePrefixesFilter list
  • and it removes the first match found in the BarcodePrefixesFilter list from the beginning of the “scanString”; or returns an empty string if no matches were found

Ok, that’s great… now, why does it do that? Where does this code express the needs of the business; the understanding of why those characters need to be removed from the front of the scanned string? How much additional code am I going to have to read to understand the context of this code? How long will it take me to understand what functionality this code facilitates, from a functional view of the system?

… easy to read is not the same as easy to understand. Be sure your code is understandable, not just readable; and the only way to do that is for someone else to look at your code and work with it.


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 Analysis and Design, AntiPatterns. Bookmark the permalink. Follow any comments here with the RSS feed for this post.
  • http://ralfw.blogspot.com Ralf Westphal

    Great observation – but how? What could the developer of the class do to make it easier to understand? Add comments? Or come up with a different design altogether?

    -Ralf

  • http://simpleprogrammer.com John Sonmez

    Good point.

    I was thinking the same thing that Ralf commented, but then I realized why I didn’t understand it well. It was easy enough to read one line of code, but sometimes even one line of code needs to be pulled out into a method to add understanding.

    For example:

    public bool IsSatisfiedBy(string item)
    {
    return BarcodePrefixesFilter.Where(item.StartsWith).Any();
    }

    if much clearer, IMO as:

    public bool IsSatisfiedBy(string item)
    {
    return StartsWithAnyMatchingPrefix(item);
    }

    Now you could also move that understanding up one level by changing the API so that IsSatisfiedBy is actually named StartsWithAnyMatchingPrefix, but I don’t know enough about the context two say whether there should be 2 layers of abstraction or just 1 to insulate the how from the what.

    Just my 2 cents. Great point though.

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

    @Ralf / @John,

    I purposely left the “answer” out of this post because it would have distracted from the point I was making. The answer is entirely contextual and dependent on a significant number of things – including knowledge of the domain, the business needs, the architecture, the use of the code in question, etc. There is far too much explaining that I would have to do to provide the answer… and on top of that, it would not leave you with questions, wondering how to solve the problem, which is partially what I wanted to do. This problem needs to be solved by every individual in their system, not by some blabbermouth (me) with a blog post. :)

  • http://simpleprogrammer.com John Sonmez

    @Derick

    Yeah, I get that, and I agree 100% with what your saying. I took a stab at clearing up a low level understanding issue, but in the bigger context, I wouldn’t have any idea without the domain knowledge.

    It makes me think about looking at my code from the perspective of someone without domain knowledge and seeing if it still makes sense though.