<CharlieDigital/> Programming, Politics, and uhh…pineapples

30Mar/12Off

Procedural vs. Structural Code

In working with my development team, one of the things I've been working on is to figure out how to get them to be more object oriented. I've written about this topic before and I continue to evolve my expression of this idea to other developers that I work with.

To me, beyond the simple textbook definition of object oriented programming, at the level of construction, what it really boils down to is expressing logic structurally as opposed to procedurally. That is the very basis of what I've been trying to communicate and trying to understand how to impart that on other developers.

As a basic example, let's consider a user management API that has three operations for add, update, and delete.  Suppose I want to send a sequence of actions to this API.  We see this type of pattern of interaction all the time.  Assume we have a repository class:

public class UserRespitory
{
    public void Add(User user)
    {
        Console.Out.WriteLine("Added!");
    }

    public void Delete(User user)
    {
        Console.Out.WriteLine("Deleted!");
    }

    public void Update(User user)
    {
        Console.Out.WriteLine("Updated!");
    }
}

I've left the implementation of the actions intentionally simple.  Now assume that we have an operation and a user class like so:

public class Operation
{
    private readonly string _type;
    private readonly User _user;

    public Operation(string type, User user)
    {
        _type = type;
        _user = user;
    }

    public string Type
    {
        get { return _type; }
    }

    public User User
    {
        get { return _user; }
    }
}

public class User
{
    private string _username;

    public User(string username)
    {
        _username = username;
    }
}

Consider a use case where we must build an API to support bulk operations.  The typical implementation pattern that I will encounter will look more or less like this:

private static void Main(string[] args)
{
    List<Operation> inputs = new List<Operation>
    {
        new Operation("Add", new User("Charles")),
        new Operation("Update", new User("Steve")),
        new Operation("Delete", new User("John"))
    };

    UserRespitory repository = new UserRespitory();

    foreach (Operation operation in inputs)
    {
        switch (operation.Type)
        {
            case "Add":
                repository.Add(operation.User);
                break;
            case "Update":
                repository.Update(operation.User);
                break;
            case "Delete":
                repository.Delete(operation.User);
                break;
            default:
                throw new InvalidOperationException();
        }
    }
}

This is pretty much textbook procedural code and, as a consultant, this is pretty much the code I expect to see (and have seen countless times) on projects.  Of course, this is a pretty harmless case; in measuring the cyclomatic complexity and maintainability index of this code using Visual Studio's built-in analysis tools, I get values of 7 and 67 for each respectively.

Already a 7 with just a basic implementation.

Cyclomatic complexity is a good tool because it gives us a quantitative metric of code quality that is indifferent to "style" or any qualitative prejudices that individual developers might have -- it's a pretty good starting point for determining where to focus your time in terms of code reviews and refactoring.  With that said, a value of 7 is certainly not bad (the NIST guidelines recommend a limit of 10 for methods), however, I think we've all encountered this before: if-else's nested under switch's nested under if-else's nested under for-loops and so on.  When I see this pattern, I usually encounter it in the 20-30 range (and I've seen it in the 100+ range, sadly (an extract from a 1078 line method with cyclomatic complexity of 136 in production code -- I couldn't zoom out far enough to capture the whole thing)) for cyclomatic complexity simply because the developer doesn't know when to stop the madness.

Well, let me correct that last statement: when a developer starts down this path and once they've gone deep enough, there is simply no way that they can recover from the madness without structurally refactoring the code; the switch-case traps you into a cycle of cancerous code unless you can find a different way of representing the same logic in a more modular, orthogonal manner.  To me, this is really the heart of understanding object oriented programming.  How we achieve this -- in this case -- is quite simple, really.  For starters, we make the operation abstract:

public abstract class Operation
{
    private readonly User _user;

    public Operation(User user)
    {
        _user = user;
    }

    public User User
    {
        get { return _user; }
    }

    public abstract void Execute(UserRepository repository);
}

We also add an abstract method called Execute which every inheriting class must implement -- it's here that we've captured the essence of the switch-case in a much more structural, object-oriented manner.  Instead of a switch statement, we build the same logic structurally into the code by using three classes:

public class DeleteOperation : Operation
{
    public DeleteOperation(User user) : base(user) {}

    public override void Execute(UserRepository repository)
    {
        repository.Delete();
    }
}

public class AddOperation : Operation
{
    public AddOperation(User user) : base(user) {}

    public override void Execute(UserRepository repository)
    {
        repository.Add();
    }
}

public class UpdateOperation : Operation
{
    public UpdateOperation(User user) : base(user) {}

    public override void Execute(UserRepository repository)
    {
        repository.Update();
    }
}

The abstract method now gives us a way of specifying a different action for each "case" by creating another class.  Now our main code can be updated:

private static void Main(string[] args)
{
    // Structural representation of the logic
    List<Operation> inputs = new List<Operation>
    {
        new AddOperation(new User("Charles")),
        new UpdateOperation(new User("Steve")),
        new DeleteOperation(new User("John"))
    };

    UserRepository repository = new UserRepository();

    inputs.ForEach(i => i.Execute(repository));
}

What is our end result?

Improvements in maintainability and cyclomatic complexity

The change in the code itself was very minor; we performed a slight-of-hand and simply moved the three logical branches in the switch into three separate classes instead with each class inherently representing one of the switch cases.  In doing so, however, the cyclomatic complexity for the main program halved to 3 and the the maintainability index increased 6 points, even with such a barebones change.

The overall maintainability index of the entire solution increased 6 points.  Our cyclomatic complexity for the solution did increase, but that's because we've added three new classes which adds to the sum.

There are several other advantages that are easy to extend from this point:

  1. It's easier to add new operations.  Instead of adding another case, we simply add another operation class and inherit from the abstract base class.  The logic for that particular operation -- for example, business logic or setup before the operation is invoked -- can be encapsulated in the new class instead of in the case.
  2. It's easier to add more logic around the operations.  Because we've removed the invocation from the switch, we can avoid a giant if with deeply nested code as the logic around the operations change.  With a class for each operation, we can add operation specific logic to the class instead.  If we need to add common logic, we can use the base class.
  3. It's easier to read.  I think the code is easier to read simply because the level of nesting is now greatly reduced.  McConnell writes in Code Completed, 2nd Edition:

    "The smaller part of the job of programming is writing a program so that the computer can read it; the larger part is writing it so that other humans can read it." (McConnell, 733)

  4. It's less prone to user error.  By removing the switch and getting away from a string comparison, we've made the solution more resilient to input errors.

Overall, it's a very minor change in the structure of the code (check it out, even the LoC is the same), but even in this most basic of scenarios, it yields clear objective and subjective improvements.  While this is a very basic example of a very specific case, the underlying idea here -- structural representation of logic and flow -- is the real essence of object oriented programming.  The intentionally simplistic example is meant to (hopefully) help surface this concept, but the general idea of refactoring to structural representations is applicable to many different use cases, scenarios, and coding patterns.

Code samples: SampleProcedural.zip and SampleStructuralCode.zip

Posted by Charles Chen

Filed under: Dev Comments Off
Comments (0) Trackbacks (0)

Sorry, the comment form is closed at this time.

Trackbacks are disabled.