My Disagreement With Elegant Objects
We don't have to agree on everything to learn and grow from each other. This is exemplified for me with the Elegant Objects by Yegor Bugayenko.
Elegant Objects being a book read about 6 months after an OOBootCamp from Fred George crystallized a few remaining pieces that the boot camp only narrowed down.
I've spent more than a year driving on the concepts presented by Fred and Yegor into a style called MicroObjects. If you're reading this, you know about it. :)
I've been promoting and lending Elegant Objects at work. I have around ten copies I loan to whoever would like to read it. In the discussions we have about the book, a few things come up where I don't fully agree, or fully drive, on it.
I think this is normal and a positive thing. Disagreement encourages discussion, which increases understanding. This helps us make better decisions. Following the rules blindly will not get you into a position to make decisions that the rules don't cover.
So let's disagree!
There's three things that I don't fully agree with in Elegant Objects.
Even in just these three, it's not complete disagreement, just different views and approaches. Which, personally, I love to encounter. We can learn from each other. Having this difference in opinion forces me to learn from myself to - Which is a huge reason I write posts like this.
Why do I do it differently?
I want to find better ways to do things. I can't do that unless we all can see the reasons for doing what I currently do. If I can't defend it, or get convinced - I'll abandon a position in favor of one I can argue for.
Here's the primary points that I disagree with from Elegant Objects and why.
It's all about prefixes
This is my biggest disagreement with Yegor.
He says that the method prefix of get
or set
is the problem because of it's treats the object like a data bag instead of an object. It's an effect of anthropomorphizing the object, we want to treat it respectfully.
I absolutely agree with this. Anthropomorphizing helps us look at how we're treating the object and if it's how we'd treat a person.
To avoid getting into why we anthropomorphize, we do, it helps and this is the conclusion of that. We don't "GET" the data, we ask for the information.
The examples he provides demonstrate this perfectly
class Cash{
private final int value;
public int dollars(){ return value; }
}
//instead of
class Cash{
private final int value;
public int getDollars(){ return value; }
}
And this I disagree with, strongly. I consider it wrong. It's really bad advice for the promotion of object oriented programming.
Simply dropping a prefix doesn't make the databag into an object. It's still a databag. The reason this matters is that it will still be used as a data bag.
Allowing data access like this enables developers who don't understand to continue writing procedural code "without getters".
The biggest factor that drives me to think this way is from Fred.
Why?
When asking for a variable the class holds stop and ask, "Why?"
"Why do you want it?"
"What are you going to do with it?"
The answer will be some kind of behavior. The caller will need to ask for the data
public string FormattedPhone(IUser user){
string rawPhone = user.RawPhone();
}
then they'll need to do something with it
public string FormattedPhone(IUser user){
string rawPhone = user.RawPhone();
string formattedPhone = this.FormattedPhone(rawPhone, this._formatter);
}
And whatever BEHAVIOR is being done must be known by the class asking for the data. We're FORCED into procedural code, FORCED out of object oriented programming doing this.
After whatever behavior, the result is probably going to have some behavior done with it as well. In this case we'll just return it.
public string FormattedPhone(IUser user){
string rawPhone = user.RawPhone();
string formattedPhone = this.FormattedPhone(rawPhone, this._formatter);
return formattedPhone;
}
Something else now has gotten raw data and is forced to act procedurally. I think Elegant Objects has something about this, just not finding it.
Even when we're not violating my version of "No Getters" by not returning a variable we hold - we're violating single responsibility. Anywhere that wants a formatted phonenumber is going to have to implement their own version of it, or KNOW somewhere else that has it and tightly couple to it.
There's behavior here - formatting a phone number. It's a simple one-line-able thing
public string FormattedPhone(IUser user){
return this.FormattedPhone(user.RawPhone(), this._formatter);
}
The consumer is forced to know a lot because it's forced to do procedural code because it has to explicitly implement the behavior it needs.
This is the wrong thing to do to an object. Let the object know what it wants, and give it the collaborators that know how to do it.
Reset
Let's go back to asking for the raw phone number (see, I'm not saying "get").
Why?
Why do you want it?
What are you going to do with it?
To format it
Sounds like a useful thing, let me do it.
Let the object with the data control the behavior around the data.
In the previous example we had to have a method that did the formatting, but let's ask the holder of the data to do it for us...
public string FormattedPhone(IUser user){
return user.FormattedPhoneNumber();
}
This method is now a pass through... We encapsulated the behavior and removed the need for an entire layer in this functionality. The caller that needs a formatted phone now just needs to interact with the user
object. That behavior is now ENCAPSULATED with the data. When you don't let your data out, you force the behavior to be next to the data.
Following this strictly, the IUser
interface is going to EXPLODE in size. A far violation of Elegant Object's 5 method recommendation (which I support, though am inclined to say it's too many). Then we look at the questions being asked of data and create objects around the data to expose that behavior.
Instead of string IUser#FormattedPhone
we'd have IPhoneNumber IUser#PhoneNumber
and string IPhoneNumber#Formatted()
, or similar.
This is my major disagreement with Elegant Objects. No Getters should be never returning the value you hold.
The bottom line is that getters and setters are a terrible anti-pattern in OOP.
I fully agree with Yegor... just for a different reason.
Never return the value you hold.
Don't use new
outside of secondary ctors
I agree with this. I've written about this a few times here, here, and here.
I don't agree with the how.
I often reference this practice as "No new
s inline". We don't instantiate our collaborators where we use them. If it's an object the class interacts with, it's passed in. Either as a param in the method, or into the Primary Ctor.
Yegor's example for the NYSE
works well for me. It's how I do my class collaborators.
class Cash{
Cash(int value){
this(value, new NYSE());
}
Cash(int value, Exchange exch){...}
}
Here I fully agree with the idea. It's manual dependency injection, works fantastic.
What I don't agree with is where he takes this for the Request
example.
(it's on page 179, I'm not typing it out)
I understand what's being sought, and achieved, with the Mapping
construct. I don't see it being useful enough to warrant the abstraction and indirection that it brings along.
The expectation that you can "inject our own implementation of Mapping
" and have it be meaningful is flawed. For testing - If we do that then we're not testing the actual code. If we need it for another class to unit test, we Fake it.
If any other class "knows" what needs to be created and returned by injecting a new Mapping
- that class KNOWS too much.
There is a very distinct difference between how my class instantiation looks and how Yegor's looks. In his style of having a huge nested new construct, being able to inject makes some sense. I'd argue to have a SimpleRequestRequests
and a OtherRequestRequests
object which know the Mapper
to use, but that's biased with my style.
I get it - I think it hinders further evolution of looking at objects as things to interact with, that may simply know something.
Boolean Results
I still use Is
and Has
for boolean methods. Yegor's recommendation is to not. Instead of IsEmpty
, have just Empty
. Instead of Has
... ok, he doesn't adress the Has
prefix, but same idea. HasSubObject
.... SubObjectPresent
?
I agree with the idea. I'm not a fan of the prefixing. I'm also lazy. Getting the naming just right to avoid the prefix... is more effort than I've found it to be worth.
This is one I agree is valuable and ... it's my own laziness and lack of experience removing the boolean prefix that has me say that it's not one to worry about. There's bigger battles to fight (like appropriate no getter) that I'm happy to throw this one out while other aspects are worked on.
Use aspect-oriented programming
I don't like the use of annotations to drive behavior. It gets too easy to have a single method do too much because it's visibly very small... and not even part of the method's line count.
Aspect Oriented Programming (AOP) has some appeal, and I've written a few of these types of simplifications because it is appealing. It hides code. It hides what's actually happening and how it's doing it.
It's just another class doing another single behavior. In the Elegant Object example, it's a class doing multiple things.
int attempt = 0;
while(true){
try{
return http();
}catch(IOException ex){
if(attempt >= 2){
throw ex;
}
attempt++;
}
http()
is some other method doing the network request.
This difference in opinion probably stems from the difference in style of development. I can see how it fits in Yegor's projects. We do things differently, so end up with different preferences. I'm definitely advocating the style that works with MicroObjects. :)
I'd do this closer to
int attempt = 0;
while(true){
try{
return _request.http();
}catch(IOException ex){
if(attempt >= 2){
throw ex;
}
attempt++;
}
In this example we have a seperate object responsible for performing the http()
behavior. All this class would be responsible for is retrying. It'd basically be the same behavior as assigned to the @RetryOnFailure
aspect, but is more explicit about what's happening, right there in the code.
What if we need to handle more than an IOException
? How will the AOP behave? Can you stack them? What do we do?
When I have the code available to be modified, I can get it to do exactly what I need, exactly how I need it. Most of the time I encounter these AOP practices, there's a one off that needs it different - and suddenly you're in a situation where you have multiple styles to accomplish the same thing. This increases the complexity of the code base making it harder to maintain.
Summary
That's where I primarily disagre with Yegor in Elegant Objects. I think it's all about the style of development we do. We find different things work better in that environment. My disagreements are ones that start to force code into MicroObjects, my preferred style. :)
I enjoy sharing my views and hearing other inputs as that's how we can help each other get better!