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