Object Calisthenics #2: Bringing Order to Chaos by Pierre

Object Calisthenics, a set of programming exercises designed to improve code quality and object-oriented design, can be a powerful tool when refactoring existing codebases.

To see how they work, Yoan and I decided to do the exercise on our own, without influencing each other.

If you haven't read it, here's the link to the explanation of the exercise to which this article is a response:

This article chronicles my experience applying these principles to a Lord of the Rings-themed project, highlighting the steps taken and the reasoning behind each decision.

Step-by-Step Methodology

Understanding the Project and Creating a Safety Net

Before diving into refactoring, I took the time to understand the project structure and functionality. The codebase consisted of a console application that managed characters in the Fellowship of the Ring, their weapons, and their movements between regions.

To ensure I didn't inadvertently break existing functionality, I created a non-regression test using the Verify library. This test captured the console output of the program, providing a safety net for the refactoring process:

[Fact]
public async Task BigGoldenTest()
{
    var stringWriter = new StringWriter();
    Console.SetOut(stringWriter);
    ....
}

With this safeguard in place, I was ready to begin the refactoring process.

Don't Abbreviate: Improving Code Readability

The first rule I tackled was "Don't abbreviate." The Character class had properties with cryptic names like N, R, and W. I used Rider's refactoring tools to rename these properties to more descriptive names:

public sealed class Character
{
    public string Name { get; set; } // Formerly N
    public string Race { get; set; } // Formerly R
    public Weapon Weapon { get; set; } // Formerly W
    public string Region { get; set; } = "Shire"; // Formerly C
}

This simple change immediately improved the code's readability and maintainability.

Keep All Entities Small: Redistributing Responsibilities

Next, I focused on the FellowshipOfTheRingService class, which was handling multiple responsibilities. To adhere to the "Keep All Entities Small" principle, I decided to redistribute these responsibilities to more appropriate classes.

ESRDTFYGUHIJO

Wrap All Primitives and Strings: Introducing Value Objects

To further improve the domain model, I implemented the "Wrap All Primitives and Strings" rule.

public record Name
{
    public string Value { get; }

    public Name(string value)
    {
        if (string.IsNullOrWhiteSpace(value))
            throw new ArgumentException("Name cannot be null or empty");
        Value = value;
    }
}

These value objects encapsulate validation logic and make the domain model more expressive. They also prevent invalid states, as it's now impossible to create a Name with an empty string or a Damage with a negative value.

For concepts with a limited set of possible values, I opted to use enums instead of string properties. This was particularly applicable for Race and Region:

public enum Region
{
    Shire,
    Rivendell,
    Moria,
    Lothlorien,
    Mordor
}

By wrapping these primitives, I centralized validation logic and made the domain model more expressive.

Don't Use the ELSE Keyword: Simplifying Control Flow

To adhere to the "Don't Use the ELSE Keyword" rule, I removed unnecessary else statements using Rider's "Remove redundant else" option. This simplified the control flow and made the code more concise:

if (exists)
    throw new InvalidOperationException("A character with the same name already exists.");
members.Add(character);

First Class Collections: Encapsulating Member Management

To implement the "First Class Collections" rule, I created a Fellowship class to encapsulate the list of members and related operations:

public class Fellowship
{
    private List<Character> Members { get; } = new();
    
    public void AddMember(Character character)
    {
        // Implementation
    }

    public void RemoveMember(Name name)
    {
        // Implementation
    }
}

This change allowed me to move member management logic out of the FellowshipOfTheRingService class, further adhering to the "Keep All Entities Small" principle.

With the basic structure in place, I continued to refactor and simplify the code. This included moving logic into appropriate classes, such as adding a MoveToRegion method to the Character class:

public class Character
{
    // Other properties and methods

    public void MoveToRegion(Region region)
    {
        if (Region == Region.Mordor && region != Region.Mordor)
            throw new InvalidOperationException("There is no coming back from Mordor.");
        Region = region;
    }
}

I also simplified loops and conditions, making use of LINQ where appropriate:

public void UpdateCharacterWeapon(Name name, Name newWeapon, Damage damage)
{
    var character = fellowship.Members.FirstOrDefault(c => c.Name == name);
    character?.ChangeWeapon(new Weapon(newWeapon, damage));
}

No Getters/Setters/Properties: Balancing Encapsulation and Practicality

While I didn't strictly adhere to the "No Getters/Setters/Properties" rule, I did take care to limit the exposure of setters. This approach helps maintain encapsulation while still allowing necessary access to object state:

public class Character
{
    public Name Name { get; }
    public Race Race { get; }
    public Weapon Weapon { get; private set; }
    public Region Region { get; private set; }

    // Constructor and methods
}

One Dot per Line: A Natural Outcome

Interestingly, the "One Dot per Line" rule was largely satisfied as a natural consequence of the other refactoring steps. By breaking down complex operations and moving logic into appropriate classes, the code naturally became more aligned with this principle.

Undone rules

In this refactoring exercise, I didn't strictly adhere to two Object Calisthenics rules: "No Classes with More Than Two Instance Variables" and "Only One Level of Indentation per Method".

This decision was based on a personal preference and a pragmatic approach to code design. While these rules can offer benefits in certain contexts, I found them overly restrictive for this particular project.

The goal of Object Calisthenics is not to apply rules at all costs, but to use them as part of a broader strategy for improving code quality. In the case of the "Only One Level of Indentation per Method" rule, a good alternative is to lean towards more functional programming practices, where tasks are broken down into separate methods.

The Result

We now end up with the code divided into 2 classes: one for managing members, and one for display + a display part. I could have gone further and taken the display part out of Fellowship and kept it in the service to separate the uses.

public class Fellowship()
{
    public List<Character> Members { get; } = new();

    public void AddMember(Character character)
    {
        if (character == null)
        {
            throw new ArgumentNullException(nameof(character), "Character cannot be null.");
        }

        if (Members.Any(member => member.Name == character.Name))
        {
            throw new InvalidOperationException(
                "A character with the same name already exists in the fellowship.");
        }

        Members.Add(character);
    }

public void RemoveMember(Name name)
{
    var characterToRemove = Members.FirstOrDefault(character => character.Name == name);
    if (characterToRemove == null)
    {
        throw new InvalidOperationException($"No character with the name '{name}' exists in the fellowship.");
    }

    Members.Remove(characterToRemove);
}

public void MoveMembersToRegion(List<Name> memberNames, Region region)
{
    foreach (var name in memberNames)
    {
        MoveMemberToRegion(name, region);
    }
}

private void MoveMemberToRegion(Name name, Region region)
{
    foreach (var character in Members.Where(character => character.Name == name))
    {
        character.MoveToRegion(region);
    }
}

public List<Character> GetMembersInRegion(Region region)
{
    return Members.Where(character => character.Region == region).ToList();
}
}

public class FellowshipOfTheRingService
{
    private Fellowship fellowship = new();

    public void AddMember(Character character)
    {
        fellowship.AddMember(character);
    }

    public void RemoveMember(Name name)
    {
        fellowship.RemoveMember(name);
    }
    
    public void UpdateCharacterWeapon(Name name, Name newWeapon, Damage damage)
    {
        foreach (var character in fellowship.Members.Where(character => character.Name == name))
        {
            character.ChangeWeapon(new Weapon(newWeapon, damage));
            break;
        }
    }

    public void MoveMembersToRegion(List<Name> memberNames, Region region)
    {
        fellowship.MoveMembersToRegion(memberNames, region);
    }

    public void PrintMembersInRegion(Region region)
    {
        var charactersInRegion = fellowship.GetMembersInRegion(region);
        if (charactersInRegion.Any())
        {
            Console.WriteLine($"Members in {region}:");
            foreach (var character in charactersInRegion)
            {
                Console.WriteLine($"{character.Name} ({character.Race}) with {character.Weapon.Name}");
            }
        }
        else if (charactersInRegion.Count == 0)
        {
            Console.WriteLine($"No members in {region}");
        }
    }

    public override string ToString()
    {
        var result = "Fellowship of the Ring Members:\n";
        foreach (var member in fellowship.Members)
        {
            result += $"{member.Name} ({member.Race}) with {member.Weapon.Name} in {member.Region}" + "\n";
        }
        return result;
    }
}

Conclusion

Applying Object Calisthenics to an existing codebase is a challenging but rewarding process. It forces you to think deeply about your code's structure and design, leading to more maintainable and expressive code. However, it's important to note that not all rules need to be followed dogmatically.

The key takeaway from this experience is that Object Calisthenics provides valuable guidelines for improving code quality, but they should be applied judiciously. The goal is not to blindly follow rules, but to use them as a tool for creating more robust, maintainable, and expressive code.

By focusing on clear naming, proper encapsulation, and appropriate distribution of responsibilities, we can significantly improve our codebase. The journey of refactoring is ongoing, and these principles provide a solid foundation for continuous improvement in our software design practices.

The source code can be found here: