Don't work too hard

Don’t write more code than is necessary to get the job done. 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.

Code Gardening
Don't work too hard

About this series

Posts in this series

Comments

Code GardeningDon't work too hard

About this series

Posts in this series

Comments

Code Gardening
Don't work too hard