To keep our code at Google in the best possible shape we provided our software engineers with these constant reminders. Now, we are happy to share them with the world.
Many thanks to these folks for inspiration and hours of hard work getting this guide done:
- Jonathan Wolter
- Russ Ruffer
- Miško Hevery
Also thanks to Blaine R Southam who has turned it into a pdf book.
Flaw #1: Constructor does Real Work
Warning Signs
- new keyword in a constructor or at field declaration
- Static method calls in a constructor or at field declaration
- Anything more than field assignment in constructors
- Object not fully initialized after the constructor finishes (watch out for initialize methods)
- Control flow (conditional or looping logic) in a constructor
- Code does complex object graph construction inside a constructor rather than using a factory or builder
- Adding or using an initialization block
Flaw #2: Digging into Collaborators
Warning Signs
- Objects are passed in but never used directly (only used to get access to other objects)
- Law of Demeter violation: method call chain walks an object graph with more than one dot (.)
- Suspicious names: context, environment, principal, container, or manager
Flaw #3: Brittle Global State & Singletons
Warning Signs
- Adding or using singletons
- Adding or using static fields or static methods
- Adding or using static initialization blocks
- Adding or using registries
- Adding or using service locators
Warning Signs
- Summing up what the class does includes the word “and”
- Class would be challenging for new team members to read and quickly “get it”
- Class has fields that are only used in some methods
- Class has static methods that only operate on parameters
48 responses so far ↓
Guide to Writing Testable Code | Miško Hevery // Nov 24, 2008 at 8:52 am
[...] Guide: Writing Testable Code [...]
Michael Tsai - Blog - Guide: Writing Testable Code // Nov 24, 2008 at 9:32 am
[...] Miško Hevery: To keep our code at Google in the best possible shape we provided our software engineers with these constant reminders. Now, we are happy to share them with the world. [...]
Nice blog post.
In Flaw #3: Brittle Global State & Singletons
you mention “# Adding or using registries” is a warning signal. However, most applications have some objects that need to be accessible application wide, or at least based on some context. How would we manage such objects without a registry?
In a Java web server, we would have to use registries and static attributes to manage user sessions, application scope, etc. I am not sure if these could be managed without registries and static attributes.
Overall, I do agree that usage of static attributes and registries should be minimized, but perhaps it is not possible to write an application without using them at all.
In “Flaw #4: Class Does Too Much” you mention “# Class has fields that are only used in some methods” as a warning signal. Again I agree, but at the same time there are cases where such usage would be valid. For instance in a DAO each attribute would not be used in all the methods. Also in UI components, each method will not make use of all the instance attributes.
Thanks for sharing your insights. It would be really nice if you can mention exceptions to the above in some future blog post.
This is fantastic.
I’ve been trying to bring all of your blog posts and presentations together to present to my team – but this goes beyond any of that.
@Parag,
To answer your first question about registries. Registry, Context, ServiceLocator, call it what you want is a problem. First How do you get a hold of it? Global variable?
Global variables are a problem:
http://misko.hevery.com/2008/08/25/root-cause-of-singletons/
http://misko.hevery.com/2008/08/17/singletons-are-pathological-liars/
Locators are a problem:
http://misko.hevery.com/2008/10/21/dependency-injection-myth-reference-passing/
And the solution is Dependency Injection even if it is a widely needed object:
http://misko.hevery.com/2008/10/21/dependency-injection-myth-reference-passing/
Java web servers are a problem because Sun messed up the API and they force you to have no argument constructor. You have two choices:
1) user Jetty which allows you to construct your own servlets and do proper Dependency Injection: http://code.google.com/p/unit-test-teaching-examples/source/browse/trunk/src/di/webserver/ServerBuilder.java?r=6
2) If you insist on using Tomcat, or something else which does not allow you to do dependency injection, than you are right you need to have global variable where the world can communicated through. However, this variable should be closely guarded and only the constructor of the servlets are allowed to call it. Something like this
class MyServlet {
// No Arg Constructor required by API
MyServlet() {
this(global.getA(), global.getB(), …);
}
// Dependency Injected constructor where
MyServlet(A a, B b, …) {
…
}
}
We will use the DI constructor for testing and only use the no arg constructor with the nasty global variable in production. The key is that ONLY the no arg constructor is allowed to talk to the Global variable! NO ONE ELSE is allowed.
It is TOTALLY possible to write an application without using global state. I have always write my apps in such a way.
– Misko
Thanks for working so hard on the topic and producing such excellent content! It makes my life as a testing evangelist much easier when I am able to point to such great resources.
cheers Thomas
Excellent guide! I find sometimes it is hard to respect the law-of-demeter, I have a case where Object A needs a collaborator B that can only be determined at runtime.
I cannot easily wire because the collaborator varies based on 2 runtime parameters passed to A. For this reason I have to inject a locator C in A and pass it the args to resolve an instance of B.
class A {
// injected C
private C c;
doWork(args) {
B b = c.resolveB(args);
b.helpWithTheWork();
}
}
C is mockable as is B. So how would you enhance the testability of this code?
The main difficulty here is that B is scoped to a transaction and points to a different database based on the runtime args.
B is in fact a javax.persistence.EntityManager that points to a different persistence unit based on the runtime args.
The lifecycle of the transactional entitymanagers are managed by the J5EE container.
@Guillaume,
LoD is a symptom. It is not a cause. The cause is that your object responsibilities are not ideal. So looking at the symptom it is impossible to come up with a fix until you can understand all of the pieces. This is what makes the LoD so hard to obey. The place where the mistake is made and where it is felt are not the same place.
Normally, I break my application into “zones” each zone are objects which have the same runtime. First are objects which live for the duration of application the singletons (Lower case “s” non global). These includes a factory which know how to create the next zone. The HttpRequest Zone. Those objects live for the duration of the HttpRequest and than they are garbage collected. Now a long lived object should never ask for a short lived object. So a singleton should not know about Request objects. But a request object can know about the longer lived (ie the singleton).
In your example the c.resolveB method seams to be in the wrong location. I think class A is long lived, but should be short lived. So when A is constructed it gets handed in B and the LoD problem goes away. If you tell me that there are reasons why A needs to be long lived, than I think you are mixing object concerns and A has too many responsibilities.
Basically you need to have some object FactoryA which is responsible for bridging the life-cycle of the two objects types. Each objects should be in the business of either graph construction or doing work. In your case your Class A does both, wiring and work.
http://misko.hevery.com/2008/07/08/how-to-think-about-the-new-operator/
Re: the warning about use of ‘new’ in a constructor. I understand we should pass in references to collaborators, but what about a Class that manages a List ? In the constructor one might use “new List()” to initialize the List. If the List is intrinsic to the class, why would it be folly to create it (empty) in the constructor? That way all your List based methods would at least have an empty List to work with. Is this the exception to the rule?
@Clinton,
Excellent question! Lists and many other objects are an exception to this rule. Here is the discussion more in depth: http://misko.hevery.com/2008/09/30/to-new-or-not-to-new/
Regarding Flaw #4: What does “Class has static methods that only operate on parameters” exactly mean? This leaves room for interpretation.
Misko,
Thanks alot for your advice. You are right A is long-lived and there is no logical reasons why it should be. I use both EJB3 container and spring container. A, B & C are configured in spring, however A is injected in an EJB3.
One of the reasons why we use EJBs is because we found the tx management in spring to be awkward and prefer the EJB3 tx management approach. Since the EJB3 dependency injection capabilities are limited we still want to use Spring.
As you probably already know spring eagerly wires all objects on startup and in order to support smaller scopes than the singleton (”small s”) scope it uses proxies. Spring doesn’t come out of the box with a transaction scope even though I know it is possible to create custom scopes and use them.
So as you can see in my usecase its not trivial to make Spring (myfactory) to create A when the transaction starts and wire B at that point.
Do you think Spring falls short with managing object scopes?
PS: Your article and testing guide are great! Thanks for sharing the knowledge.
I find this statement interesting:
“Class has static methods that only operate on parameters.”
I strongly believe in keeping methods shorter than a screen. To this end, I create a decent amount of helper methods. These are great candidates for static methods if they don’t operate on fields and eclipse will help you realize that by making your method italic.
Further, taking apache commons io as an example, I’d much rather write:
IOUtils.closeQueity(is);
than:
try { is.close(); } catch (IOException e) {/**/}
A lot of what I do is writing parsing routines or routing via object types to appropriate controllers (via MVC).
“Class has static methods that only operate on parameters”
makes no sense to me (no more or less than TDD)
Hi!
I’m a big fan of your testing related articles.
I’ve been wondering about the Flaw #1: Constructor does Real Work. What if I have a class, that needs to do something before it’s really functional and properly set up. Who’s responsibility is it to make instantiated classes to do their initial setup work?
Say I have a RSSChangeListener which does something when a RSS feed is updated. RSSChangeListener’s constructor gets passed in a singleton RSSRepository. To start listening to the already existing RSS feeds, I need to ask RSSRepository for feeds the user has added. Then I start listening to changes to those RSS feeds. Of course later on the user can add more RSS feeds and my class needs to listen to those too.
How is the initial setup done? Whos responsibility is it to start listening to the already added RSS feeds? Constructor would be a nice place, because that way object is ready for use after being constructed by the factory.
Of course this problem exists with RSSRepository also. When does the RSSRepository read stored feeds from disk? And who tells it to do it? Whoever does that, must then tell every object relying on RSSRepository that now they can access the repository, because now it has its state properly set up.
If the factory initialization time is not the right time, what is? Do I need a separate class which just kicks every object created by the factories and tells them to initialize them to a sane state? With complex hierarchies of objects I’d need to make sure I kick my objects in the correct order and maintain that order when code changes.
Do you have any thoughts or suggestions? Thanks!
2008 November 29 - Links for today « My (almost) Daily Links // Nov 28, 2008 at 9:59 pm
[...] Misko Hevery publishes his Guide: Writing Testable Code [...]
b.l.o.g. » Blog Archive » Bookmarks for November 28th // Nov 29, 2008 at 8:09 am
[...] Guide: Writing Testable Code | Miško Hevery – [...]
For those who want to digest this information in smaller chunks, here it is as a daily RSS feed:
http://feedchopper.ning.com/index.php/main/feed/showUrl?id=162617833
Bert Lamb » links for 2008-11-30 // Nov 30, 2008 at 2:02 am
[...] Guide: Writing Testable Code | Miško Hevery (tags: programming software tips development unittest testing) [...]
Blog Xebia France - Revue de Presse Xebia // Dec 1, 2008 at 10:54 am
[...] Hevery, coach Agile à Google, partage sur son blog quelques-unes des recommandations et alertes que Google suggère à ses développeurs afin de [...]
Well written! However, static methods can be very important for reuse and logical reasons!! Couldn’t you just write each call to a static method inside an overridable protected method (which only purpose is to call the static method)? That way you have the possibility to override it for testing purposes…
Dev Blog AF83 » Blog Archive » Veille technologique : MySQL, Symfony, Javascript, Performance, Éditeurs de texte // Dec 2, 2008 at 8:19 am
[...] http://misko.hevery.com/code-reviewers-guide/ : un guide pour écrire du code plus facile à tester [...]
I do agree that testability is a very important concern. In fact, in static vs. dyanmic debate I am usally the one who brings the “testing is more important than type checking” argument.
However, I think that hard rules such as these (I just saw a post calling them “coding formulas”) will always end up beating their own purpose. See for example, the comments by Eric and Petteri.
Here is a more elaborate discussion of such formulas: http://javadots.blogspot.com/2008/12/software-formulae-are-illusive.html
Guide to Writing Testable Code / CODEISPOETRY // Dec 10, 2008 at 6:37 pm
[...] Extern gefunden: It is with great pleasure that I have been able to finally open-source the Guide to Writing Testable Code. [...]
Static Methods are Death to Testability // Dec 15, 2008 at 10:49 am
[...] Guide: Writing Testable Code [...]
Good stuff, Misko.
Interfacing with hard-to-test third-party code // Jan 4, 2009 at 12:26 pm
[...] Guide: Writing Testable Code [...]
Woow good stuff.. Any chance you’d make an offline version of this or something? Like perhaps a pdf or an ebook.. I want this on my iPhone..
@Edward, I was thinking about turning it into hard copy / book. But I can’t seem to find the time.
Something like this would really benefit from adding of what you should do instead of the bad things.. “x is bad, instead do y”
Misko Hevery’s Blog: Guide: Writing Testable Code | How2Pc // Jan 7, 2009 at 8:46 pm
[...] this slightly older (Nov 2008) but useful post to Misko’s blog, he takes a look at a few common flaws that you should avoid in writing up [...]
Hi, Misko. Excelent blog post.In “Flaw #4: Class Does Too Much”, when you mentioned “Class has static methods that only operate on parameters” as a warning sign, how can we deal with classes that operate upon dates and strings, such as doing tasks like formatting and parsing? Those classes are common to have a lot of static methods that operate only on parameters..Is there a way DI can help with it?
Rafael, see how they’ve done it in Joda time, web-search for DateTimeFormat. Note that the static methods that are there are only factories, they don’t actually do anything.
jo-zone.com // Jan 20, 2009 at 1:11 pm
[...] The Google guide to software: This guide makes so much sense and illustrates beautifully how a well intentioned software [...]
A rule of thumb and a silver bullet « Luke Halliwell’s Weblog // Jan 22, 2009 at 1:13 pm
[...] doubt there are more: check out this excellent article by Miško Hevery of [...]
Writing Testable Code « Dark Views // Jan 30, 2009 at 10:00 am
[...] 1. December, 2008 at 17:21 | In Software | Tags: TDD Just stumbled over this article: “Writing Testable Code“. Apparently, it’s a set of rules which Google uses. While I’m not a 100% fan of [...]
Guide to testability is now downloadable // Mar 9, 2009 at 7:45 pm
[...] Guide: Writing Testable Code [...]
Guide to testability « A Place In The Queue // Mar 12, 2009 at 2:02 pm
[...] to testability Pubblicato su Vari by acerisara su Marzo 12th, 2009 Segnalo questo bel paper che spiega alcune linee guida per scrivere codice testabile. Semplice, conciso e con [...]
Thanks for your great materials.I wonder which version you are using for guice?In document, there are many @Provies annotation but I can’t find it from guice.
Extreme Enthusiasm » Blog Archive » Not wishful thinking // Mar 17, 2009 at 12:38 am
[...] My point is that the most important thing a programmer needs is to learn how to write clean, testable code. Selenium and similar tools are useful, but secondary; what a programmer needs the most is to learn [...]
AS3 Design anti-Pattern, Singleton, 反对……单态模式（单件，单例） // Mar 24, 2009 at 5:11 am
[...] Guide: Writing Testable Code 也把Singleton不利测试的一种Warning [...]
tony arkles blog » A Fantastic Testing Resource (Thanks Misko Hevery) // Apr 30, 2009 at 6:39 pm
[...] Guide: Writing Testable Code [...]
Bashar’s Blog » A must read for people who wish to do unit testing ! // May 3, 2009 at 11:26 pm
[...] can be found on Miško Hevery’s website or you can directly download the pdf from this [...]
Mark’s Testblog » Blog Archive » Testable code happens to be better designed code - …for these are testing times, indeed. // May 10, 2009 at 6:40 pm
[...] another month, a workmate found Misko Hevery’s guide to testability and the penny dropped. Like google’s testing blog logo — it was like switching on a [...]
Hi,I watched two of your presentations on google and just read twice your pdf guide on writing testable code. I want to thank you, it was very enlightening. I intend to do a presentation to the developers in my company on that subject.Take care !
@ Bashar,
Glad to hear that you found it useful.
Not a comment rather a question: Is there a test explorer tool for C code too as you have for Java?
@Tushar,
There is someone working on it but progress is slow.
Leave a Comment