Everyone seems to think that they are writing OO after all they are using OO languages such as Java, Python or Ruby. But if you exam the code it is often procedural in nature.
Static Methods
Static methods are procedural in nature and they have no place in OO world. I can already hear the screams, so let me explain why, but first we need to agree that global variables and state is evil. If you agree with previous statement than for a static method to do something interesting it needs to have some arguments, otherwise it will always return a constant. Call to a staticMethod() must always return the same thing, if there is no global state. (Time and random, has global state, so that does not count and object instantiation may have different instance but the object graph will be wired the same way.)
This means that for a static method to do something interesting it needs to have arguments. But in that case I will argue that the method simply belongs on one of its arguments. Example: Math.abs(-3) should really be -3.abs(). Now that does not imply that -3 needs to be object, only that the compiler needs to do the magic on my behalf, which BTW, Ruby got right. If you have multiple arguments you should choose the argument with which method interacts the most.
But most justifications for static methods argue that they are “utility methods”. Let’s say that you want to have toCamelCase() method to convert string “my_workspace” to “myWorkspace”. Most developers will solve this as StringUtil.toCamelCase(“my_workspace”). But, again, I am going to argue that the method simply belongs to the String class and should be “my_workspace”.toCamelCase(). But we can’t extend the String class in Java, so we are stuck, but in many other OO languages you can add methods to existing classes.
In the end I am sometimes (handful of times per year) forced to write static methods due to limitation of the language. But that is a rare event since static methods are death to testability. What I do find, is that in most projects static methods are rampant.
Instance Methods
So you got rid of all of your static methods but your codes still is procedural. OO says that code and data live together. So when one looks at code one can judge how OO it is without understanding what the code does, simply by looking at the relationship of data and code.
class Database {
// some fields declared here
boolean isDirty(Cache cache, Object obj) {
for (Object cachedObj : cache.getObjects) {
if (cachedObj.equals(obj))
return false;
}
return true;
}
}
The problem here is the method may as well be static! It is in the wrong place, and you can tell this because it does not interact with any of the data in the Database, instead it interacts with the data in cache which it fetches by calling the getObjects() method. My guess is that this method belongs to one of its arguments most likely Cache. If you move it to Cache you well notice that the Cache will no longer need the getObjects() method since the for loop can access the internal state of the Cache directly. Hey, we simplified the code (moved one method, deleted one method) and we have made Demeter happy.
The funny thing about the getter methods is that it usually means that the code where the data is processed is outside of the class which has the data. In other words the code and data are not together.
class Authenticator {
Ldap ldap;
Cookie login(User user) {
if (user.isSuperUser()) {
if ( ldap.auth(user.getUser(),
user.getPassword()) )
return new Cookie(user.getActingAsUser());
} else (user.isAgent) {
return new Cookie(user.getActingAsUser());
} else {
if ( ldap.auth(user.getUser(),
user.getPassword()) )
return new Cookie(user.getUser());
}
return null;
}
}
Now I don’t know if this code is well written or not, but I do know that the login() method has a very high affinity to user. It interacts with the user a lot more than it interacts with its own state. Except it does not interact with user, it uses it as a dumb storage for data. Again, code lives with data is being violated. I believe that the method should be on the object with which it interacts the most, in this case on User. So lets have a look:
class User {
String user;
String password;
boolean isAgent;
boolean isSuperUser;
String actingAsUser;
Cookie login(Ldap ldap) {
if (isSuperUser) {
if ( ldap.auth(user, password) )
return new Cookie(actingAsUser);
} else (isAgent) {
return new Cookie(actingAsUser);
} else {
if ( ldap.auth(user, password) )
return new Cookie(user);
}
return null;
}
}
Ok we are making progress, notice how the need for all of the getters has disappeared, (and in this simplified example the need for the Authenticator class disappears) but there is still something wrong. The ifs branch on internal state of the object. My guess is that this code-base is riddled with if (user.isSuperUser()). The issue is that if you add a new flag you have to remember to change all of the ifs which are dispersed all over the code-base. Whenever I see If or switch on a flag I can almost always know that polymorphism is in order.
class User {
String user;
String password;
Cookie login(Ldap ldap) {
if ( ldap.auth(user, password) )
return new Cookie(user);
return null;
}
}
class SuperUser extends User {
String actingAsUser;
Cookie login(Ldap ldap) {
if ( ldap.auth(user, password) )
return new Cookie(actingAsUser);
return null;
}
}
class AgentUser extends User {
String actingAsUser;
Cookie login(Ldap ldap) {
return new Cookie(actingAsUser);
}
}
Now that we took advantage of polymorphism, each different kind of user knows how to log in and we can easily add new kind of user type to the system. Also notice how the user no longer has all of the flag fields which were controlling the ifs to give the user different behavior. The ifs and flags have disappeared.
Now this begs the question: should the User know about the Ldap? There are actually two questions in there. 1) should User have a field reference to Ldap? and 2) should User have compile time dependency on Ldap?
Should User have a field reference to Ldap? The answer is no, because you may want to serialize the user to database but you don’t want to serialize the Ldap. See here.
Should User have compile time dependency on Ldap? This is more complicated, but in general the answer depends on wether or not you are planning on reusing the User on a different project, since compile time dependencies are transitive in strongly typed languages. My experience is that everyone always writes code that one day they will reuse it, but that day never comes, and when it does, usually the code is entangled in other ways anyway, so code reuse after the fact just does not happen. (developing a library is different since code reuse is an explicit goal.) My point is that a lot of people pay the price of “what if” but never get any benefit out of it. Therefore don’t worry abut it and make the User depend on Ldap.
45 responses so far ↓
The statement “the answer depends on weather” made me smile
I have an on-topic question, too.
Where does a method for storing a user in the database belong? Should it also be a method of User, or of some DatabaseHelper class?
@Thomas,
That is kind of complicated, I personally believe that User should not know how to persist or materialize itself. The reason for this is explained here http://misko.hevery.com/2009/05/05/the-problem%20-with-active-record/ and here http://misko.hevery.com/2008/09/30/to-new-or-not-to-new/. You may have lots of persistent strategies (SQL, XML, Serialization) and as a result it would be mixing of responsibilities to include them in user. It is true that the save method will have high affinity to user fields, but in this case the save method is kind of marshaler. It only knows that it needs to take data from here and move it there. I think Hibernate got it right.
Your example with authentication could be rewritten so that User will not have compile-time dependency on Ldap, thus be reusable in another project. But with introduction of one more interface.
interface Authenticator {
boolean authenticate(String username, String passwd);
}
class LdapAuthenticator implements Authenticator {
LdapAuthenticator(Ldap ldap);
boolean authenticate(String username, String passwd);
}
class User {
// other members and methods
Cookie login(Authenticator auth){
if( auth.authenticate(user, password) )
return new Cookie(user);
else
return null;
}
}
User only depends on the Authenticator interface, not the particular implementation.
I recently stumbled upon one of your articles and have been addicted ever since. Coming from a PHP and JavaScript background I find it hard to come across decent articles about good OO design. Your posts are quite often exceptionally easy in terms of examples but teach great lessons. Thanks a lot, keep up the good work:-D
Yes, it could be rewritten that way, but:
“My point is that a lot of people pay the price of “what if” but never get any benefit out of it. Therefore don’t worry abut it and make the User depend on Ldap.”
So, if you ever need to re-use the code in another project, do the refactoring required then. Another way of saying this is “you won’t need it”:
http://c2.com/xp/YouArentGonnaNeedIt.html
Hey Misko,
Nice article, one question though:
Cookie login(Ldap ldap) {
if ( ldap.auth(user, password) )
return new Cookie(actingAsUser);
return null;
}
It seems to me this would be nicer if it was in a tell don’t ask style
void login(Ldap ldap, CookieAcceptor cookieAcceptor) {
if ( ldap.auth(user, password) )
cookeiAcceptor.new(new Cookie(actingAsUser));
return cookieAcceptor.thereWasAnAuthProblem(user);
}
I like the tell don’t ask style here because now you’re not requiring the client to do a null check, and you’re really defining the interactions of this method. There’s the disadvantage of another interface, but I think it pays off
I’m php/js guy myself, but you really do a great job, eye-opening articles! I learned a lot from you and thank you very much! I wish you didn’t mention so many Java related solutions, but I’m not whining, I just need to Google a few words like Guice or Ldap…
@Zach,
From a testing point of view asking is better since you have fewer objects to deal with. It comes down to not having to need to mock out CookieAcceptor. (With a strange name like Acceptor).
This code is mainly for demonstration purposes.
@Thoer,
sorry, I know Java the best, but I also know JavaScript, Python and Ruby, so I should really branch out. But at the end of the day it would be the same lesson, just different syntax. So far the rules of good design and testability transcend language syntax differences.
Hi Misko,
I agree there’s a little more to testing, but I think returning null is problematic. And in terms of OOifying some code it seems to me that using IoC is appropriate in your example. Anyway I have a more detailed response over at my blog: http://zdsbs.blogspot.com/2009/08/returning-null.html
@Zach
Maybe I’m missing your point, but right now I can’t imagine a situation where I don’t want to check right after the call to login() if the login was successful and branch accordingly.
Tomas
@Thomas
You don’t necessarily need the branching… I mean this is a toy example, but my thinking these days is returning null effectively allows you to return a second type from you method. And on the client side when you say if (cookie == null) is equivalent to saying if (cookie instanceof failedAuth).
so another way to do it is instead invert the control so the login method calls the proper methods on the client, loginsucceeded(cookie) or loginFailed(user). I went into more detail on that blog post I linked to.
BTW i never thought of null checks like instance of checks before… but I think they kind of are.
Readings | Prajwal Tuladhar's Blog // Aug 2, 2009 at 12:11 am
[...] Using only classes do not make your code object oriented [...]
@Zach,
I agree with you.
@Zach,
Yes, this is a toy example and returning null is a bad idea. You should never return null in production code.
I like your blog, but sometimes I wish it was more testing centric, and less Java centric.
Java is kind of OO… I guess? Kind of sort of? That’s what Sun would have us think anyway.
Static methods in particular are implemented pretty stupidly in Java. You can’t pass around a class object in Java (well, without resorting to reflection), so they can’t be used polymorphicaly like they can in… real OO languages.
You can’t pass around first class functions in Java like you can in a real functional language either. So that removes another kind of polymorphism from static methods as well.
However, in most other languages functions or class methods are more useful. So discussions of the weakness of static methods just devolve into “why the heck are you still using Java anyway?”
From a testing perspective, Python is a much better language.
“Example: Math.abs(-3) should really be -3.abs()”
Not in Java, no. That would parse as -(3.abs()) according to the precedence rules. Interestingly, Scala seems to have a special case for it, 3.abs works, -(3.abs) works, (-3).abs works but -3.abs gives a compile error.
If you define an appropriate extension method in C#, -3.Abs() gives -3.
Your idea about recognising which class a method belongs to by examining the source seems pretty good, but what happens when there’s no difference; a method accesses objects from two classes quite equally?
Common Lisp supports OOP far more completely than Java, and in that language methods execute with objects as arguments. There is no ‘this’.
Further, a reasonable rule is to try to keep items that will change together together, and moving a method to wherever the object it most accesses is defined seems to thwart that. You could end up having a User who knows how to display itself on screen, serialise itself, etc.; hardly a separation of concerns.
I think you mean that static methods are death to mocks, not to testability. You can certainly test a static method, and you can certainly test code that uses static methods. If the static method is tested, there’s no reason not to test code that uses the method without mocking the method out.
I so wish that Java would never started with the whole static non-sense, but I guess it was just an necessary evil to attract masses of procedural programmers and allow them to slowly transform into (in most case half-baked) OO programmers.
It would be great to have a way of extending 3rd party classes in Java language, but that’s not going to happen.
IMO Ruby’s way of opening classes is mind-blowing *and* equally dangerous. Mainly because it makes the type system very fragile. It works great for small scripts, but in larger applications it can cause lots of headaches, especially later in the application lifetime.
On the other hand the implicit type conversion as implemented in Scala seems to be equally powerful, but a lot safer.
Thanks for the great post.
Thanks for excellent post again, Misko!
I completely share your point of view regarding proper OO usage and mostly unnecessary worries about code resusability.
Regarding exceptional situation handling I’d like to point to my favourite approach, described in http://www.oracle.com/technology/pub/articles/dev2arch/2006/11/effective-exceptions.html
I think it is the best strategy if you stick with imperative programming – in other cases Option could be a better fit for you.
Why not creating a class like UserDAO and putting the LDAP logic into it? This way you can remove the compile time dependencies between User and LDAP. I think that it pays the price to create such a class, since it requires little effort and you can make the User class reusable.
igorbrejc.net » Fresh Catch For August 4th // Aug 4, 2009 at 4:04 am
[...] How to think about OO [...]
Really nice Misko, I have seen a lot of people talking about rescue the OO principles and so forth, but I you gave us some concrete examples, and that is really enlightening.
I’m curious how to reconcile the idea that static methods are evil with the elegance of something like the C++ STL, where the whole point is that algorithms (i.e. sort, find) are separate from the containers. I guess from a pure OO perspective, the STL is quite poor, but it’s one of the best-designed libraries I’ve seen in any language.
Hi Misko,
from nearly one month I stumbled upon your blog and I’m reading all your articles. I found them very formative and very enlightening.
Thank you! Keep writing please!
Hi Misko,
I wanted to join these people that praise your blog. I’m regularly checking for new posts since they are very relevant and enlightening as Indirit said.
Really great to read, as a high quality book in sequels. Very different from most of worthless blogs around.
Keep it going.
Very good…I’m a big fan.
I disagree with using the keyword extends…at all. In my opinion there is no reason to ever use this keyword. Extends is *worse* than using static.
If you think about it, extends is a form of static reuse or static function calling. The difference between extends and implements is that you have the ability to call into another class’s (the parent’s) methods.
This calling relationship is static…it cannot be changed at run time. Any time you have a function that will always call the same implementation but that implementation does not reside in your class, it feels static. Unlike, for example, the object graph or hierarchy, the extension based inheritance graph can never be changed and forms a static binding between classes.
Instead please use the keyword implements (above, you could have had an interface User, with a NormalUser as well as a SuperUser.
For further examination of this topic, please examine the dark hippo/dark jedi explanation:
http://www.berniecode.com/writing/inheritance/
“Static methods are procedural in nature and they have no place in OO world.”
So would you not call C++ an OO language?
If so what about a template class with a static method, does that have no place is OO?
Right lads tell STL, Boost and Loki there libraries are no good because Misko does not understand the need for static methods.
M isko, U have a little bug:
is: user.isAgent
should be: isAgent
in third listing.
Here’s what I don’t understand – lets say User has some other functionality attached to it, other than login, say a logout function. Then let’s say we pass user into a function logoutIfFlyingArtichoke which relies on the logout function but not the login function, so we’re breaking your principle of passing in only what a function needs by giving logoutIfFlyingArtichoke access to the login function.
Furthermore, to properly test logoutIfFlyingArtichoke, we’d need to pass in a subclass of User which throws an exception if login is called, and for another function which uses login but not logout you’d need a subclass of User which throws an exception on a call to logout. Seems like it’s more difficult to test too.
@Val Karpov
You can easily avoid the problems you’ve mentioned by applying Interface Segregation Principle.
@Przemek
That can quickly get really bad really quickly – you can end up with an interface for every function defined in your class, public class User implements Login, Logout, etc. which basically leaves you trying to hack Java into being a functional language.
@Val Karpov
If you want to properly test logoutIfFlyingArtichoke, you often want to mock its collaborators to check if only proper functions were called. That leaves you two options IMO, each with its problems. You either subclass User and throw exceptions (poor solution IMO) or use ISP (and worry about functional approach in Java: not a problem for me). Of course, you can make logout static, but how are you going to mock it to prevent its side-effects and to check whether it was called?
Finally, I don’t know your case, but I think worrying about unexpected calls to a method from another one is unnecessary in many cases.
Misko, I understand the problem with static methods. I also understand that another OO principle you advocate is separation of object creation from object logic. So you have object factories, and all the “if” statements can be relegated there allowing you to avoid these conditionals in your object logic by using polymorphism (as described in this post in the case of the User classes).
When you do all this, what I end up seeing a lot of times is a static factory method (often inside a dedicated static – or might as well be static – factory class). What’s the proper approach here? Specifically, where does the User factory go in your example so that we don’t end up with a static factory method and/or class?
@John,
how can they be static? A factory should have a get() method with zero arguments (most of the time). This means that it will always create the same thing (unless there is global state, or same parameters get passed in through the factory constructor.). Are your objects calling factory.get()? (bad) or are they simply asking for the dependencies in the constructor? (good). Look at GUICE and how it solves the factory problem.
– misko
Misko, you often stress, that objects should not call factory.get(). In most cases it’s better to ask for collaborators in constructor.
However, what if I really need to create objects as a part of request processing? Sometimes I have not enough data to create them beforehand.
Example: user submits his postal address via a web form. Depending on the address I will display an optimal shipping strategy (each strategy can execute different business process). The most natural way for me to implement it is to call shipping=factory.get(address), and then shipping.display()
That would be however calling factory from my objects, that you don’t advise. How would you approach such problem?
Hey!
Wonderful as usual. For completness could you please further explain something.
You got rid of the “If”s by using polymorphisim (creating the SuperUser etc). My question is … How would you use this in code. Could you show (if possible) how you would structure the code that uses each of the user types. Will there stil lbe an “If” or “switch” that returns new SuperUser.. or new User??
I hope my question made sense.
Thank you for sharing this wealth of information.
Hi Misko,
You mentioned that some languages offer extension methods. The language I work with, C#, indeed does offer this. However, it should be noted that you can’t mock extension methods, so you’ll need to manually write a test double for dependencies with extension methods (assuming you need to call such methods in your tests).
… of course now that I think about it, perhaps extension methods are best used only with newables, and if an injectable needs to be extended it gets Decorated.
Mikso,
Below is an example of what I am talking about. When the factory method is implemented, I often see it implemented as a static method (and often in what might as well be a static class). Look at the examples in this article to see what I was getting at:
http://en.wikipedia.org/wiki/Factory_method_pattern
@John,
The issues in this example is that a static method is fixed at compile time and will always return the same kind of object. This does not help you in testing as you want to return a different kind. For example you want AlwaysAuth class rather than LDAPAuth class. Static method can not make this decision unless there is global state, which is very bad way to solve this problem. So static factory method buys you nothing.
– Misko
Making Zend_Auth more “Object Oriented” | Aviblock.com // Oct 29, 2009 at 6:15 pm
[...] controller. I knew there had to be a better way. So after a lot of thought, pondering, reading, and inspiration, this is what I’ve come [...]
Dependency Injection vs Factory at CodesOfMyLife // Mar 21, 2011 at 8:05 am
[...] method in factory. So what is the problem we may ask? Well it introduces hidden dependency. This [1] articulates it pretty well. In summary, “the caller of the static factory is permanently [...]
Return Null value on the issue - Open News // Jan 30, 2012 at 11:19 pm
[...] always feel a method returns a null value problem. When reading Misko Hevery on how to think about OO after the blog post, reminds me of the [...]
Thanks for the post.It helps me.
Leave a Comment