I give the orders around here!
I have been programming since I got my first Commodore 64 as a nine year old kid. However I still think I have so much to learn when it comes to writing good code.
In my quest to improve I have learnt many different languages. Most of them are predominantly object oriented (OO).
What suprises me though in most of the books, magazines and on-line articles that I have read is the amount of horrible OO code used in examples.
The most violated principle I see is the “Tell, Don’t Ask” principle. This principle states that an object should tell other objects what to do and not query other objects for state to make decisions (querying objects for state to make decisions is also referred to as ‘Feature Envy’). In Object Oriented Programming an object is defined as having state and operations that manipulate that state.
In his book ‘Holub on Patterns: Learning Design Patterns By Looking At Code’, Allen Holub includes a section in chapter one titled ‘Why getter and setter methods are evil’. He also contributed it as an article to JavaWorld. It should be a “must read” for any OO developer.
I have worked with developers whose first step after declaring attributes on a class is to add getters and setters. The JavaBean specification has a lot to answer for in promoting this culture. It was deemed a good way to write re-usable modular components but the world has come a long way since then.
Writing classes with getters and setters leads to procedural code. Logic that operates on data retrieved from getters ends up spread out across an application and is often duplicated across an application (violating another principle DRY – Don’t Repeat Yourself). This leads to unmaintainable code as any change to a class causes a ripple effect throughout the application.
By exposing the data you also prevent easy refactoring of the class because any change to the way an attribute is represented affects any other object that has access to that attribute.
Another side-effect of violating the Tell, Don’t Ask principle is that your tests end up being very state-based with stubs requiring a lot of expectations. This can make it very hard to understand what is actually being tested.
A third principle you will probably end up violating is the Principle of Least Knowledge (or LoD – Law Of Demeter). This law can be summarised as follows:
A class should only talk to its immediate friends and don’t talk to strangers.
By adding getters to your class you end up writing code like:
if (person.getAddress().getCountry() == "Australia") {
This violates the Law of Demeter because the caller is too intimate with Person. It knows that they contain an Address and that Address contains a country. It could be written like this instead
if (person.livesIn("Australia")) {
This does not violate the Law of Demeter as the caller is only talking to their immediate friend Person and they have no knowledge of the internals. From a Tell Don’t Ask perspective this is better because the logic for determining if the person lives in Australia is hidden within Person and if we decided to change how the country value is represented in Person it would not affect any other classes that depend on that value.
The other side effect of writing the code this way is that it displays the intent much clearer. That will be another topic that I will be touching on soon.
Some things to keep in mind to avoid violating the Tell Don’t Ask principle.
- Think before hitting the hot keys in your IDE exercise the ‘5 Whys’ and ask yourself why you need to add a getter or setter in the first place.
- Do not ask objects about their state, make a decision, then tell them what to do.
- Operations belong with the class that owns the data.
Further Reading
A friend of mine is a strong advocate of this and has come up with the
East Oriented approach that explains this principle brilliantly.
Tell Don’t Ask and the effect on testing From Steve Freeman and Nat Pryce, authors of Growing Object-Oriented Software, Guided by Tests.
crustyoldev 
The “anemic domain model” (as Fowler calls it) is one of my pet hates (as you well know) so yay for blogging about this (and yay for the blog generally!)
However: given you’re blogging about ‘the horrible code used in OO examples’, you probably shouldn’t have your example code use a String to represent a ‘Country’ datatype. Because that’s an equally common (and egregious) mistake. 🙂
Agree with you on using a string to represent the country, but I just wanted the example code to read better 😛
I don’t agree with this at all.
When my objects in Ruby have getters and setters, they give other objects more information. But they are not bundled together at all, because all they provide is INFORMATION for other objects to make use of.
Why would this be bad? They are not coupled together because this INFORMATION can come from ANY OTHER SOURCE too – from a flat file. Or a database. Or autogenerated.
And in all those cases it must be validated anyway.
The problem is if your getters provide access to the internal state of your object.
For example if you had an Account object that had a getBalance() and setBalance() methods that allowed another object to query and modify the balance. Now you don’t want to let anyone debit any amount they like and put the account into negative, so a better approach would be to have a debit(amount) method on Account that could validate that there are sufficient funds to debit and then update the balance accordingly.
This encapsulation also gives you the ability how the balance is represented without affecting any callers that are dependent on the debit() method.
I like the last example – but I would like to hear your thoughts as to avoiding classes with hundreds of methods while following that pattern. I only have bitter experiences about this… this quantitative complexity I couldn’t really escape. The textbook answer is to break up the class into smaller chunks, right? However I am yet to see an elegant breakup of a larger class (=having great many of very closely interrelated methods).
I’ll try to answer your question the best I can but I am unsure of the full context of your question.
If you are worried that maybe you might want to find a person that lives in a country and maybe on a particular street then the person object is probably not the best place for this type of method. In this case I would either have a PersonRepository that is responsible for retrieving Persons and have methods on it that either have specific finder methods e.g. byStreet() or take some set of Crtieria (i.e. a more functional style). Another possibility is that the PersonRepository returns a Persons object that effective wraps a collection of Person and provides filter methods to only select particular Person objects from the result.
Also, if your class has lots of attributes then maybe you are missing an abstraction and the class is responsible for too many things (violating the Single Responsbility Principle).
I don’t see this as a getter/setter problem but more of a visibility problem. In the case of getting the address and setting the country the address should have been a private property with the getter having protected visibility. I do understand the point of the arguement but getters/setters are not the direct issue it’s really about reducing coupling.
Protected visibility is always bad. You should Google “Fragile Base Class” to understand why. Maintainability suffers whenever you expose any internal implementation details because you can’t change those details without affecting the “client” (using or derived) classes. Use protected only for Gang-of-Four Template Methods, never for fields and never for accessors (there’s no practical difference between the two).
Getters often do reveal state but who says they have to? Are you suggesting that all methods should return booleans? Because if you aren’t, then the methods that don’t, could easily be renamed to be getters.
Yes you could break it down into true Command/Query separation and have either void methods or methods that return a boolean value. Refer to the link to East Oriented code for an example of this approach.
Pingback: Commonly Violated Principles in Objected Oriented Design - How To Research
I wish to show my appreciation to you for rescuing me from this
particular condition. As a result of researching through the online world and finding
basics which were not helpful, I thought my life was well
over. Being alive without the solutions to the issues you’ve resolved as a result of your entire write-up is a critical case, as well as those which could have adversely damaged my entire career if I hadn’t encountered your web page.
Your own talents and kindness in dealing with all the pieces was crucial.
I am not sure what I would’ve done if I hadn’t come across such a stuff like this.
It’s possible to at this point look ahead to my future. Thanks a lot very much for the impressive and result oriented guide. I won’t hesitate to
recommend your blog to any person who ought to have direction about this subject
matter.
Do you think this is a topic that could be written into a guidebook?
If you extend it to more themes into different segments,
maybe you can produce it into a compact ebook to publish online
and then subsequently publish it on Amazon?
I’m impressed, I have to admit. Seldom do I come across a blog that’s both educative and entertaining, and let me tell you, you’ve hit the nail on the head. The problem is something too few people are speaking intelligently about. Now i’m very
happy I came across this in my hunt for something regarding
this.