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.
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:
We compared our results here:
If you want to see Yoan's implementation:
Have a goat day ๐
Join the conversation.