The application of code rules is often conveyed as unidirectional and one-size-fits-all. In other words, when we learn to develop, we tend to read that there is only one way to produce quality code, and only one way to implement rules or patterns.
With this series of articles, we aim to prove that this is not the case, and that trying to achieve this goal is a serious mistake. To this end, Yoan and Pierre have applied the rules of calisthenic objects to a low-quality code base to compare our results.
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 is a summary of the discussions we've had about our results, and the questions that have arisen. We believe that the methodology for producing code, and the thinking behind it, is as important to understand as the end goal. By pushing ourselves to understand other implementations of the same rules, we learn more than by viewing a single point of view.
Yoan's feedbacks on Pierre's implementation
What I Appreciated
It seems we both started with Characterization tests
using Verify
and followed a similar refactoring path, so I definitely appreciate your approachâwhether itâs great minds or just torturous ones thinking alike! đ
Points of Difference and Questions
No Classes with More Than Two Instance Variables: You mentioned not strictly adhering to the rule of limiting classes to two instance variables.
How do you decide when it's beneficial to relax this rule, and what criteria do you use to balance between strict adherence and practical design?
- Pierre: I'm not implementing it at the moment for one simple reason: I haven't yet identified any cases where I would have felt the real benefits of this rule. The way I work is basic: I need to find myself in the case that a rule solves to tell myself that it's being used. For the moment, this rule seems to be a more restricted version of the Wrap All Primitives and Strings
rule, which brings the construction of many more objects.
No Getters/Setters/Properties: Your approach to limiting the exposure of setters struck a good balance between encapsulation and practicality.
How do you decide when to expose certain properties versus encapsulating them entirely within methods?
- Pierre: By default, I try never to expose properties. In other words, when I build my object, I first set it as an attribute. Then, when I want to access a value, I ask myself why. If I see that I'm breaking the Tell, Don't Ask
principle, then I integrate the operation into my class. If I'm making a copy (e.g. I'm moving from my domain layer to my HTTP exposure, then I expose the fields to make sure I'm not exposing my domain). This is the simplest way I've found so far.
Value Objects and Records: You used record
for value objects like Name
, which I find interesting. However, using records makes it possible to "bypass" the rules encapsulated in the constructor, potentially leading to invalid states.
public record Name
{
public string Value { get; init; }
public Name(string value)
{
if (string.IsNullOrWhiteSpace(value))
{
throw new ArgumentException("Character must have a name.");
}
Value = value;
}
public override string ToString() => Value;
}
var validName = new Name("Pierre");
// Possible to instantiate invalid Name through the record
var invalidName = validName with {Value = " "};
How do you weigh the balance of risk between the simplicity of using records and the safety of enforcing constraints within a more controlled class structure?
- Pierre: That's a very good point. I tended to read everywhere that records were the solution to use without really dwelling on these details. Since the implementation, I've changed my tune. I use records very little, only on layer changes when I need a DTO.
Handling Errors with Exceptions vs. Either: I noticed you chose to keep handling business errors with exceptions, while I used the Either
monad.
Did you consider a monadic approach, or did you find that exceptions provided a clearer or more maintainable solution in this context?
- Pierre: The more I develop, the more I love using monads to manage my error cases, the best known being the Result
that you see in all the videos. But I also use it to build my business objects and services, and I see a real benefit in simplifying code writing. For this exercise, I felt that using it added complexity without adding any real value.
Passing Domain Objects to the Service: I noticed that your refactored design involves passing domain objects
directly to the service layer
.
public class FellowshipOfTheRingService
{
// Character, Name, Damage, ... are all Domain Objects (Entity or Value Object)
public void AddMember(Character character)
public void RemoveMember(Name name)
public void UpdateCharacterWeapon(Name name, Name newWeapon, Damage damage)
public void MoveMembersToRegion(List<Name> memberNames, Region region)
public void PrintMembersInRegion(Region region)
public override string ToString()
}
How do you view this approach in terms of maintaining a clear separation of concerns, and did you encounter any challenges in ensuring that the service layer remains decoupled from the domain logic?
- Pierre: In terms of cutting, I try to use domain objects as much as possible in my services. I don't split the 2 layers. For me, the domain logic layer contains all the classes/services that don't need external elements, and the application layer is a higher-level exposure that integrates external elements (e.g. database access) and is visible to my exposure layers (e.g. HTTP API).
My end words
Overall, I found your refactoring process very pragmatic and insightful. Your approach demonstrated how Object Calisthenics can be effectively applied in a practical, real-world context, showcasing thoughtful strategies that address the challenges of refactoring while maintaining a strong focus on code quality.
Pierre's feedbacks on Yoan's implementation
Points of Difference and Questions
No Getters/Setters/Properties: You've respected this rule, as you have no getter, no property and no setter. I have the feeling that it would become even harder to keep to if you were to respect an even stricter cut, notably by taking the Logger
part out of the Character
class.
logger(destination != Region.Mordor
? $"{_name} moved to {destination}."
: $"{_name} moved to {destination} đ.");
How would you keep the class information displayed without using Getters?
- Yoan: You make a great point, Pierre. In this case, I kept the logging for compatibility, but in practice, I'd challenge the need to log everythingâwhat's the real purpose?
If logging is truly essential, I think we have two options here:
- Encapsulate Logging in Domain Entities: The class handles its own logging. We inject the logging mechanism, keeping internal properties private while ensuring the necessary information is logged (the option chosen here).
- Raise Domain Events: The entities raise events for actions like moving to a new region, with external code handling the logging. This maintains encapsulation and adheres to the principle. And just to clarify, I'm not talking about Event Sourcingâjust domain events đ.
Keep All Entities Small: Unlike me, who tends to turn my classes into records, you automatically build your classes into sealed as
public sealed class Fellowship { ... }
// but not
public class FellowshipOfTheRingService(Logger logger) { ... }
Why do you do this? And why are your domain classes sealed, but not your service?
- Yoan: I am used to seal my domain classes to ensure that the core concepts and business rules remain consistent and unaltered, which is crucial for maintaining the accuracy of the domain model.
On the other hand, I did not seal service classes like FellowshipOfTheRingService
because services (Or even Use Cases) often need to be more flexible. They might need to be extended, mocked in tests, or adapted for different scenarios. By keeping them open, I allow for that necessary adaptability while keeping the domain classes tightly controlled.
First Class Collections: We've both created a Fellowship
class to manage a list of Characters
. In your case, you've added another specific class to manipulate the list with Seq
from the LanguageExt
library.
using Characters = LanguageExt.Seq<LordOfTheRings.Domain.Character>;
What is the advantage of using this internal object rather than directly manipulating aSeq
? And do you do the same forList
?
- Yoan: The main advantage here lies in encapsulating business logic within the collection itself. This approach ensures that rules, such as preventing the addition of an existing character twice, are enforced directly by the collection, keeping the logic centralized and consistent.
Additionally, by wrapping the collection, we avoid exposing non-business methods like Add
or Remove
that come with primitive types like List
or Seq
. This helps to maintain a clean API, ensuring that only operations relevant to the domain are available.
- Pierre: I also find interesting the way you rebuild all your Fellowship
members with each modification. At first glance, I have the impression that it adds a bit of complexity (need for several constructors, need to use an assignment when adding a member...).
public Fellowship() => _members = [];
private Fellowship(Characters members) => _members = members;
private Fellowship(Characters allMembers, Characters updated) => _members = allMembers + updated;
Why did you implement this solution rather than modifying the internal Fellowship
list directly?
- Yoan: I opted for this approach to enforce immutability
, which is key to making our code more predictable
and free of side effects in the domain layer. This aligns with the principle of Functional Core, Imperative Shell
, where the core logic (domain) remains pure and functional, while any imperative actions are pushed to the outer layers (Service
layer here).
Value Objects IEquatable: For all your domain objects that seem to be value objects, you add IEquatable
, which lets you override the equality operators to simplify your comparisons.
Aren't you afraid of edge effects, especially with some objects that compare internal values and others that compare references (with the native equality comparator)?
- Yoan: I follow a convention in the domain where:
- Value Objects: Equality is determined by the values they contain. If two Value Objects have the same values, theyâre considered equal, even if theyâre different instances. This is why I use
IEquatable
âit allows me to override the equality operators to enforce this behavior. - Entities: On the other hand, equality is based on their unique Ids. Two entities are considered equal if they share the same Id, regardless of other attributes.
It mirrors the real-world concept where value objects are interchangeable if they have the same values, while entities remain unique even if their properties are the same.
This kind of conventions need to be created, shared, accepted and applied by each team member to consistently apply them. Otherwise, we would increase the risk of unexpected behaviors like mentioned in your question...
My end words
I really like the structure you've given the code, notably by adding better error handling with monads, and a more functional programming-oriented structure. By doing a final comparative analysis on our implementations, you've gone further. As for the monad part, it may add a little more complexity for the uninitiated. Your code has a larger input market, but is much more respectful of calisthenic objects.
Conclusion
Here are the results we obtained on the order of implementation of the rules for calisthenic objects:
Order | Yoan | Pierre |
---|---|---|
1 | Don't abbreviate | Don't abbreviate |
2 | Wrap Primitives and Strings | Keep All Entities Small |
3 | First Class Collections | Wrap All Primitives and Strings |
4 | No Getters/Setters/Properties | Don't Use the ELSE Keyword |
5 | Don't Use the ELSE Keyword | First Class Collections |
6 | One Dot per Line | No Getters/Setters/Properties |
7 | Keep All Entities Small | One Dot per Line |
- | Only One Level of Indentation per Method | Only One Level of Indentation per Method |
- | No Classes with More Than Two Instance Variables | No Classes with More Than Two Instance Variables |
Our two approaches have many similarities. We started by setting up a test to ensure that the changes we were about to make would have no impact. With experience, especially when refactoring has been laborious, we quickly put this type of test in place before touching a line of code.
Similarly, we've tackled the rules for calisthenic objects in almost the same order, probably due to a rather simplistic code base. Despite this, there are some differences in the way we've divided up our classes.
Pierre's conclusion
For my part, I aimed to quickly explode the class of service by adding context to the code before getting into structural changes (ELSE, One Dot etc...) that I prefer to keep to the end. My aim is to make the code more functionally pleasing before improving its readability.
I've never directly applied these rules with them in mind. Rather, they're induced in the way I develop, at least the ones I use. What interested me in this exercise was the comparison of results with Yoan. Having 2 results for a code base is an exercise that allows you to compare and take another look at the code you produce, and get the best out of it.
Yoan's conclusion
I chose to begin with Characterization Tests like Pierre to lock down the existing behavior, followed by Surface Refactoringâautomated and safe changes like removing redundant code, introducing constants, and eliminating unnecessary else
keywords. This approach helped me better understand the code before diving into deeper refactoring (Object Calisthenics and Functional Core principles). I chose not to refactor service parameters as Command Objects
to maintain the existing contract and ensure compatibility with current clients.
I really enjoyed the exercise with Pierre because it was a great opportunity to give and receive constructive feedback on code, allowing us both to grow. It reminded me of the Ten Commandments of Egoless Programmingâ"You are not your code." The purpose of a review is to uncover issues, not to criticize the person behind the code. By focusing on critiquing the code itself, we can stay positive, collaborative, and dedicated to continuous improvement.
Feedback
This is one of the reasons for the difference in the âKeep All Entities Smallâ order. Yoan doesn't apply it directly because it's induced as a side effect by his use of T.D.D (Sprout Technique
), whereas Pierre prefers to implement it quickly to limit the size of his objects.
We didn't apply the âNo Classes with More Than Two Instance Variablesâ rule, because it's a heuristic rule that allows you to question your implementation rather than a rule to be followed at all costs. It highlights the fact that methods with 20 parameters often need to be replaced by an object.
With this exercise, we wanted to prove that development should not be seen as a task with only one implementation. A requirement can be implemented in several ways, as long as these follow logical choices. As we can see here, with the same rules, we have two interpretations with 2 user workflows. The final versions meet the need, one more advanced and complex, the other simpler and less accomplished, but both correctly meet the need: to make objects calisthenic.
This is the complexity and beauty of software development.
To find out more about our results, please read our respective articles on the subject:
Have a goat day đ
Join the conversation.