Don’t write more code than is necessary to get the job done. This is another in my series of posts on Code Gardening - the practice of making small improvements as you see them while you are working.

It can be very easy to write too much code. There are a lot of reasons why this might happen; the important thing is to recognise code like this when you see it and to fix the problem on the spot.

Here’s a (recreated) example of some code designed to populate a domain object from a snippet of Xml:

public Person Load(XElement element)
{
    var result = new Person();

    var fullNameAttribute = element.Attribute("fullName");
    if (fullNameAttribute != null)
    {
        result.FullName = fullNameAttribute.Value;
    }

    var familyNameAttribute = element.Attribute("familyName");
    if (familyNameAttribute !=null)
    {
        result.FamilyName = familyNameAttribute.Value;
    }

    var knownAsAttribute = element.Attribute("knownAs");
    if (knownAsAttribute!=null)
    {
        result.KnownAs = knownAsAttribute.Value;
    }

    var birthYearAttribute = element.Attribute("birthYear");
    if (birthYearAttribute!= null)
    {
        int birthYear;
        if (int.TryParse(birthYearAttribute.Value, out birthYear))
        {
            result.BirthYear = birthYear;
        }
    }

    return result;
}

This code is straightforward enough - for each of the four desired properties (FullName, FamilyName, KnownAs and BirthYear), the appropriate element is sought and, if found, used to populate the domain result.

Unfortunately, the developer has written too much code - the creators of the System.Xml
namespace anticipated the need for this scenario and elegantly solved it right in the framework.

public Person LoadMedium(XElement element)
{
    var result = new Person()
    {
        FullName = (string)element.Attribute("fullName"),
        FamilyName = (string)element.Attribute("familyName"),
        KnownAs = (string)element.Attribute("knownAs"),
        BirthYear = (int?)element.Attribute("birthYear") ?? 0
    };

    return result;
}

The static casts automatically do the right thing, returning the value of the attribute (if available) or null if not. Better yet, because the casts turn each assignment into a single line, we can collapse everything into a single initialization statement.

Unfortunately, developers work too hard all the time without knowing it. This Xml based example is just that - an example - of the broader issue.

Comments

blog comments powered by Disqus
Next Post
It's just an Internal Tool  07 Aug 2016
Prior Post
Sharpen The Saw #9  25 Jul 2016
Related Posts
Method Archetypes  15 Nov 2016
What is it with Booleans?  08 Oct 2016
Are Boolean Return values Evil?  11 Sep 2016
Null arguments are evil  14 Aug 2016
Intention Revealing Bools  02 Jul 2016
Why Code Gardening?  25 Jun 2016
Throw Rich Exceptions  11 Jun 2016
Multiple Boolean Parameters are Really Evil  22 May 2016
Boolean Parameters are Evil  15 May 2016
Validate Method Arguments  16 Apr 2016
More code-gardening posts »
Related Pages
July 2016 archive