by Miško Hevery
So you decided to finally give this testing thing a try. But somehow you just can’t figure out how to write a unit-test for your class. Well there are no tricks to writing tests, there are only tricks to writing testable code. If I gave you testable code you would have no problems writing a test for it. But, somehow you look at your code and you say, “I understand how to write tests for your code, but my code is different <insert excuse here>“. Well your code is different because you violated one or more of the following things. (I will go into the details of each in a separate blog posts)
- Mixing object graph construction with application logic: In a test the thing you want to do is to instantiate a portion (ideally just the class under test) of your application and apply some stimulus to the class and assert that the expected behavior was observed. In order to instantiate the a class in isolation we have to make sure that the class itself does not instantiate other objects (and those objects do not instantiate more objects and so on). Most developers freely mix the “new” operator with the application logic. In order to have a testable code-base your application should have two kinds of classes. The factories, these are full of the “new” operators and are responsible for building the object graph of your application, but don’t do anything. And the application logic classes which are devoid of the “new” operator and are responsible for doing work. In test we want to test the application logic. And because the application logic is devoid of the “new” operator, we can easily construct an object graph useful for testing where we can strategically replace the real classes for test doubles. (see: How to Think About the “new” Operator with Respect to Unit Testing)
- Ask for things, Don’t look for things (aka Dependency Injection / Law of Demeter): OK, you got rid of your new operators in you application code. But how do I get a hold of the dependencies. Simple: Just ask for all of the collaborators you need in your constructor. If you are a House class then in your constructor you will ask for the Kitchen, LivingRoom, and BedRoom, you will not call the “new” operator on those classes (see 1). Only ask for things you directly need, If you are a CarEngine, don’t ask for FuelTank, only ask for Fuel. Don’t pass in a context/registry/service-locator. So if you are a LoginPage, don’t ask for UserContext, instead ask for the User and the Athenticator. Finally don’t mix the responsibility of work with configuration, If you are an Authenticator class don’t pass in a path of the configuration information which you read inside the constructor to configure yourself, just ask for the configuration object and let some other class worry about reading the object from the disk. In your tests you will not want to write a configuration into a disk just so that your object can read it in again. (see: Breaking the Law of Demeter is Like Looking for a Needle in the Haystack)
- Doing work in constructor: A class under tests can have tens of tests. Each test instantiates a slightly different object graph and than applies some stimulus and asserts a response. As you can see the most common operation you will do in tests is instantiation of object graphs, so make it easy on yourself and make the constructors do no work (other than assigning all of the dependencies into the fields). Any work you do in a constructor, you will have to successfully navigate through on every instantiation (read every test). This may be benign, or it may be something really complex like reading configuration information from the disk. But it is not just a direct test for the class which will have to pay this price, it will also be any related test which tries to instantiate your class indirectly as part of some larger object graph which the test is trying to create.
- Global State: Global state is bad from theoretical, maintainability, and understandability point of view, but is tolerable at run-time as long as you have one instance of your application. However, each test is a small instantiation of your application in contrast to one instance of application in production. The global state persists from one test to the next and creates mass confusion. Tests run in isolation but not together. Worse yet, tests fail together but problems can not be reproduced in isolation. Order of the tests matters. The APIs are not clear about the order of initialization and object instantiation, and so on. I hope that by now most developers agree that global state should be treated like GOTO.
- Singletons (global state in sheep’s clothing): It amazes me that many developers will agree that global state is bad yet their code is full of singletons. (Singletons which enforce their own singletoness through private constructor and a global instance variable) The core of the issue is that the global instance variables have transitive property! All of the internal objects of the singleton are global as well (and the internals of those objects are global as well… recursively). Singletons are by far the most subtle and insidious thing in unit-testing. I will post more blogs on this topic later as I am sure it will create comments from both sides. (see: Singletons are Pathological Lairs)
- Static methods: (or living in a procedural world): The key to testing is the presence of seams (places where you can divert the normal execution flow). Seams are essentially polymorphism (Polymorphism: at compile-time the method your are calling can not be determined). Seams are needed so that you can isolate the unit of test. If you build an application with nothing but static methods you have procedural application. Procedural code has no seams, at compile-time it is clear which method calls which other method. I don’t know how to test application without seams. How much a static method will hurt from a testing point of view depends on where it is in you application call graph. A leaf method such as Math.abs() is not a problem since the execution call graph ends there. But if you pick a method in a core of your application logic than everything behind the method becomes hard to test, since there is no way to insert test doubles (and there are no seams). Additionally it is really easy for a leaf method to stop being a leaf and than a method which was OK as static no longer is. I don’t know how to unit-test the main method!
- Favor composition over inheritance: At run-time you can not chose a different inheritance, but you can chose a different composition, this is important for tests as we want to test thing in isolation. Many developers use inheritance as code reuse which is wrong. Whether or not inheritance is appropriate depends on whether polymorphism is going on. Inheriting from AuthenticatedServlet will make your sub-class very hard to test since every test will have to mock out the authentication. This will clutter the focus of test, with the things we have to do to successfully navigate the super class. But what if AuthenticatedServlet inherits from DbTransactionServlet? (that gets so much harder)
- Favor polymorphism over conditionals: If you see a switch statement you should think polymorphisms. If you see the same if condition repeated in many places in your class you should again think polymorphism. Polymorphism will break your complex class into several smaller simpler classes which clearly define which pieces of the code are related and execute together. This helps testing since simpler/smaller class is easier to test.
- Mixing Service Objects with Value Objects: There should be two kinds of objects in your application. (1) Value-objects, these tend to have lots of getters / setters and are very easy to construct are never mocked, and probably don’t need an interface. (Example: LinkedList, Map, User, EmailAddress, Email, CreditCard, etc…). (2) Service-objects which do the interesting work, their constructors ask for lots of other objects for colaboration, are good candidates for mocking, tend to have an interface and tend to have multiple implementations (Example: MailServer, CreditCardProcessor, UserAthenticator, AddressValidator). A value-object should never take a service object in its constructor (since than it is not easy to construct). Value-objects are the leafs of your application graph and tend to be created freely with the “new” operator directly in line with your business logic (exception to point 1 since they are leafs). Service-objects are harder to construct and as a result are never constructed with a new operator in-line, (instead use factory / DI-framework) for the object graph construction. Service-objects don’t take value-objects in their constructors since DI-frameworks tend to be unaware about the how to create a value-object. From a testing point of view we like value-objects since we can just create them on the fly and assert on their state. Service-objects are harder to test since their state is not clear and they are all about collaboration and as a result we are forced to use mocking, something which we want to minimize. Mixing the two creates a hybrid which has no advantages of value-objects and all the baggage of service-object.
- Mixing of Concerns: If summing up what the class does includes the word “and”, or class would be challenging for new team members to read and quickly “get it”, or class has fields that are only used in some methods, or class has static methods that only operate on parameters than you have a class which mixes concerns. These classes are hard to tests since there are multiple objects hiding inside of them and as a resulting you are testing all of the objects at once.
So here is my top 10 list on testability, the trick is translating these abstract concepts into concrete decisions in your code.
29 responses so far ↓
None of these things are specific to testability, they’re just plain good ideas for your code – except for static methods. So what’s going on here?
The thing is, I don’t think it’s the static methods per se that are the problem, it’s when you call out from within a supposed “unit” to an external static method. You shouldn’t really be doing this anyway – you should have composed them, possibly via IOC.
Let me also point out that the problem of static methods at a high level in your call graph might actually be a problem of non-static methods at a low level in your call-graph – remember to keep your stateful/side-effecting control code at the top level of your call graph.
Oi. Some of these are common sense, but others are limiting. I’m tired of reading articles like this listing all the things you can’t do in order to test. Can’t use the new operator? Seriously?
Now I’m supposed to jump through a _lot_ of hoops to avoid ‘new’ making my app more complicated and more prone to failure.
Oh wait, just write _more_ tests for that, too! Ach.
Maybe, just maybe, the current crop of testing tools aren’t up to snuff. Instead of doing funky things in your application code, maybe somebody should figure out a smarter testing framework and let the rest of us focus on getting work done.
@Mike Johnson -> “Now I’m supposed to jump through a _lot_ of hoops to avoid ‘new’ making my app more complicated and more prone to failure.”
Well separating the ‘new’ operator is called “Dependency Inject” and judging by high number of dependency-injection frameworks out there I think it is a good idea.
Contrary to your statement it makes your app less complicated, easier to understand and hence less likely to fail, even if you don’t write any tests.
Ok, so maybe there are some items here that could stand on their own as general advice, but testing and testability don’t occur in a vacuum. I’ve found that when I focus on writing testable code, I ended up with code that is well factored.
I don’t think writing testable code is second nature for most people. So revisiting the basics can be reinforcing and in some cases enlightening, kinda like seeing a movie for the second time.
Top 10 things which make your code hard to test « Quero ser Ágil // Aug 8, 2008 at 12:12 pm
[...] 8, 2008 de Rafael Mueller No embalo do post de ontem, outro artigo do Miško Hevery com coisas que você deve evitar no seu [...]
Aren’t the titles for 7 and 8 backwards? I.E., “Favor composition over inheritance” – that makes things easier to test, not harder.
In any case, great list!
Hi Misko,
Nice article. Thanks.
I have a question. You say above in “Ask for things, don’t look for things” the following:
“So if you are a LoginPage, don’t ask for UserContext, instead ask for the User and the Athenticator.”
In this example, the User is something that (it seems to me) is not easy to inject with a DI framework as it varies, based on the current User. However, the Authenticator is something you could easily inject. So, how do design this example, when you want to use a DI framework and constructor injection?
Ben
links for 2009-04-23 | the higher you fly // Apr 23, 2009 at 2:15 pm
[...] Top 10 things which make your code hard to test (tags: development testing) [...]
Why are we embarrassed to admit that we don’t know how to write tests? // Jul 7, 2009 at 1:49 pm
[...] Here is a question which you can ask to prove my point: “How do you write hard to test code?” I like to ask this question in interviews and most of the time I get silence. Sometimes I get people to say, make things private. Well if visibility is your only problem, I have a RegExp for you which will solve all of your problems. The truth is a lot more complicated, the code is hard to test doe to its structure, not doe to its naming conventions or visibility. Do you know the answer? [...]
From The Source » Unit Testing » Mocking objects that are “new”-ed up. // Jul 14, 2009 at 9:55 am
[...] was reading this article by Misko Hevery and it reminded me of times when I use to get frustrated with unit tests because [...]
Hi,
I have a question for item 3. ‘Doing work in constructor’
If the class in question is an immutable, I assume we’re better off checking and validating the constructor parameters (say, for NPEs and failing quick with IllegalArgumentException) on the constructor rather than on every method that accesses the class members, right?
@Jorge,
not necessarily. a lot of times it goes against testability: http://misko.hevery.com/2009/02/09/to-assert-or-not-to-assert/
Hi Misko,
I have a class which given a number between 0 and 255 calculates a result and returns it. This gets called alot. I decide for efficiency purposes that I’ll pre-calculate all the answers, stick them in an array and return the result via a simple lookup.
I believe the best place to build this array would be the constructor. Is this an acceptable, exception to doing real-work in the constructor? The alternative would be to do it on the first call, which seems messier.
Changing the interface to have an initialize() method seems messier still.
Regards,
Will
@Will,
what you are describing is initializing a constant which is no different from static String lookup = “”; As long what you are creating (and its internal) references are constant, it is ok, but if it mutates (like an evicting cache) it is not ok, since order of tests execution will matter.
How to make your code testable | CodeUtopia // Aug 14, 2009 at 10:18 am
[...] things I encountered in the code base I was working with. Miško Hevery has written a good post on top 10 things that make code hard to test, which is worth [...]
OK – maybe I’ve been doing this too long and need to start wearing sandals, but how do you initialise a cache of immutable objects without a static _somewhere_?
I want to load some files off disk, chew them around a bit and save the result in memory, accessible by any part of my application that needs the data.
Is that a bad pattern?
It may make testing harder, but I reckon it produces a better end product, and for me that’s most of what the game is about.
High performant alternatives to my static dictionary on an e-card please…..
@Dermot,
No, what you want is exactly what needs to be done. But you left out the most important part from your description and that is how do you get that cache to everywhere it needs to be? Global variable aka Singleton? BAD! Dependency Injection (also a single instance but no global variable) GOOD!
In your main create a cache, and than pass it to the right place. see: http://misko.hevery.com/2009/03/30/collaborator-vs-the-factory/
– Misko
Hi Misko,
really nice article !
About the 9th “rule” (Mixing Service Objects with Value Objects)… i agree that this might make the code harder to test, but completely separating the two clearly breaks the OO principle of keeping together data and logic.
Separating data and logic brings to breaking encapsulation.
I have seen a lot of project following blindly this separation philosophy and ending up in writing procedural code under the mask of oo.
And this concern is at the heart of a whole philosophy, domain driven design.
What do you think about it ?
Miško Hevery about the benefits of testable code at geekcloud.org // Nov 18, 2009 at 9:03 am
[...] for test driven development is making sure that your code is testable. Miško Hevery wrote an article about this very topic a while back. I thought he would be the ideal person to talk to me about [...]
I never said that a value object can not have behvior, I even give HashMap as an example which has lots of behavior. Read http://misko.hevery.com/2008/09/30/to-new-or-not-to-new/ for more in depth discussion on this.
Daniel E. Renfer (duck1123) 's status on Thursday, 19-Nov-09 00:26:36 UTC - Identi.ca // Nov 18, 2009 at 5:26 pm
[...] http://misko.hevery.com/2008/07/30/top-10-things-which-make-your-code-hard-to-test/ a few seconds ago from twidroid [...]
“Singletons (global state in sheep’s clothing): It amazes me that many developers will agree that global state is bad yet their code is full of singletons.”
I concur! This was especially obvious to me in Java World. Using globals is like saying Jehova to many Java programmers. But singletons are considered a ‘pattern’ and patterns are good.
@29a,
Singletons are a pattern, which people use. But just because it is a pattern does not make it good. Singleton is an anti-pattern.
Hi,
Why some of the items are ‘do this’ (2, 7, 8) and other are ‘don’t do that’ (others)? Wouldn’t it be easier to read if all item bold sentence was a ‘don’t’ (regarding the post title) ?
Thanks,
r.
Hi Misko,
I totally agree with your article, and apply these principles in my own code.
But in the case of GUI components, it can be hard to respect the point 2. Let imagine a “Window” class, containing a lot a “Button” objects. Pressing on some buttons calls a business service which executes SQL queries.
If buttons (which are the collaborators) are instantiated in the Window constructor, it becomes very hard to test. On the other hand, it becomes hardly maintainable if the Window constructor expects an arguments list of 50 buttons, instantiated by a DI container…
How would you proceed in this case ?
Thanks !
@Killan,
Think of the view portion (Window) as a factory, whose job it is to instantiate the buttons and than wire them so that the call appropriate methods on the Controller class. This way your view has only one dependency, the controller. Yes, view becomes hard to test, but if all it does is forward the call to the controller there is nothing to test, really, so we focus on testing the controller.
Testbarkeit als Code-Metrik « techscouting through the news // Jul 21, 2010 at 12:17 am
[...] oder Unsicherheit. Dazu gibt es jedoch mittlerweile gute Quellen im Netz, beispielweise Top 10 things which make your code hard to test oder How to Think About the ?new? Operator with Respect to Unit [...]
91 Ways to become the Coolest Developer in the World « Pulkit Arora // Mar 12, 2011 at 3:09 am
[...] Learn more: Top 10 things which make your code hard to test | Misko Hevery [...]
Things I’m reading // Dec 19, 2012 at 10:19 am
[...] http://misko.hevery.com/2008/07/30/top-10-things-which-make-your-code-hard-to-test/ [...]
Leave a Comment