Writing Well-Factored Code

matt — Sat, 27 Aug 2005 12:24:00 GMT

Coding is a design activity conducted at the lowest level. Similar to analysis and design patterns aiding their respective phases of the software development process, coding patterns exist to aid the implementation phase.

Common coding patterns/techniques include:

Keep methods small, focussing on discrete tasks;
Early exit to reduce nesting;
Use private fields to reduce internal parameter passing;
Use regions as a categorisation mechanism, not to hide code; and
Use internal commenting sparingly.

Keep Methods Small, Focussing on Discrete Tasks

Implementing methods of short length facilitates functional decomposition, making it easier to debug errors and more clearly communicate the intent of code. Consider the following example:

public class Employee
{
    public void DoWork()
    {
        this.ProcessOrders();
        this.TakeBreak();
        this.SweepFloor();
    }
}

Issues associated with this technique include:

Appropriate method length;
Method naming;
Focus on allocation of responsibilities;
Reuse benefits; and
Perceived Performance Implications.

Appropriate Method Length

Well-factored methods should be 5-10 lines in length. If methods exceed this length, then further factoring is required.

There will be instances whereby decomposing methods to achieve this goal does not add value, examples being methods that perform mapping functions or rudimentary validation.

Method Naming

It is imperative to name methods that reflect the intent of the behaviour of the method. Such names should also be kept as short as possible. Appropriate naming of methods significantly reduces the need for internal commenting.

Focus on Allocation of Responsibilities

Decomposing methods into smaller functions, or responsibilities, also makes it easier to see which classes are responsible for what behaviour. Reallocation of responsibilities often results from method factoring. This is a key component of Responsibility-Driven Design.

Reuse Benefits

Factoring methods into smaller functions makes it easier for functionality to be reused/overridden/extended by subclasses. This also avoids the common problem of large methods being copied into subclasses to change a small part of the method, leading to future maintenence issues.

Perceived Performance Implications

Factoring methods into smaller functions implies an increase in the number of method invocations, which can lead to performance concerns regarding the overall increased cost of method invocation. For normal business code, this low-level concern is unfounded.

If you are writing a core function for a framework (similar to the core services provided by the .NET Framework), then the cost of method invocation may become important. To address this issue, profiling of the problem area should be conducted, then act accordingly.

Early Exit to Reduce Nesting

Exiting early from methods reduces nesting and indentation. Consider the following example:

Before:

public class Employee
{
    public double CalculateExpectedBonus(double salary)
    {
        double total = 0;

        if ((salary != null ) && (salary > 0))
        {
            if ((_name != null) && (_name.StartsWith("FRED")))
            {
                total = salary * (_weight / _height);
            }
        }

        return total;
    }
}

After:

public class Employee
{
    public double CalculateExpectedBonus(double salary)
    {
        if ((salary == null ) || (salary == 0))
        {
            return 0;
        }

        if ((_name == null) || (!_name.StartsWith("FRED")))
        {
            return 0;
        }

        return salary * (_weight / _height);
    }
}

Implementing this technique commonly involves reversing logic and reduces the need for temporary variables. Both of these factors are realised in the above example. Guard clauses are the most common application of this technique.

Methods that have a void return type can use a return; statement.

Note that the idea of early exit is somtimes disliked as methods should have one point of exit for simplicitly reasons. This concern is unfounded due to the increased complexity involved in conforming to the single exit principal, as demonstrated by the above (albeit contrived) example.

Use Private Fields to Reduce Internal Parameter Passing

Private fields can be defined to reduce parameter passing between methods internal to a class. Consider the following example:

Before:

public class Order
{
    public void Process()
    {
        ArrayList orderItems = new ArrayList();

        this.RetrieveOrderItems(orderItems);
        this.ProcessOrderItems(orderItems);
    }

    private void RetrieveOrderItems(ArrayList orderItems)
    {
        ...
        SqlDataReader dataReader = command.ExecuteReader();
        while (dataReader.Read())
        {
            OrderItem orderItem = new OrderItem(dataReader);
            orderItems.Add(orderItem);
        }
        ...
    }

    private void ProcessOrderItems(ArrayList orderItems)
    {
         foreach(OrderItem item in orderItems)
        {
            OrderItemDispatcher.Instance.Send(item);
        }
    }
}

After:

public class Order
{
    private ArrayList _orderItems;

    public Order()
    {
        _orderItems = new ArrayList();
    }

    public void Process()
    {
        this.RetrieveOrderItems();
        this.ProcessOrderItems();
    }

    private void RetrieveOrderItems()
    {
        ...
        SqlDataReader dataReader = command.ExecuteReader();

        while (dataReader.Read())
        {
            OrderItem orderItem = new OrderItem(dataReader);
            _orderItems.Add(orderItem);
        }
        ...
    }

    private void ProcessOrderItems()
    {
         foreach(OrderItem item in _orderItems)
        {
            OrderItemDispatcher.Instance.Send(item);
        }
    }
}

As the number of parameters passed between methods increases, so to does the complexity of the code. The use of private fields alleviates this issue.

Note that this technique is sometimes disliked as it becomes more difficult to identify where a given private field is referenced and modified, leading to hidden dependencies between methods. Adoption of this technique is a trade-off for simplicity. Most modern IDEs have reference identification facilities, which further alleviate this concern.

Use Regions as a Categorisation Mechansism, Not to Hide Code

Regions are intended to be used to categorise methods, not to hide code. If the latter is evident, then the code requires refactoring. Common uses of regions include categorisation of Constructors, Private Fields, Private Methods and Public Methods, as demonstrated in my earlier article Implementing the State Pattern in C#.

Generated code is an exception to this rule.

Use Internal Commenting Sparingly

Internal comments should be used sparingly, and only where the intent of a statement is not obvious. Alternatively, the preferred approach is to factor the statement into a method with an appropriate name indicating the intent.

Internal comments that state the obvious do not add value, and add 'noise' to the surrounding code. Consider the following example:

// Add the order item to the order
order.Add(orderItem);

Conclusion

Coding patterns can be applied during the initial development phase and as part of maintenence activities, and ultimately deter the onset of software entropy (a.k.a. Big Ball of Mud software architectures). The adoption of such techniques results in clearer communication that simplifies the maintainability and extensibility of code.

mattonsoftware.com : Coding

Writing Well-Factored Code