A model for improving the names of variables, fields, interfaces, classes and namespaces in a system. Practise this and more in my course, Agile Design: Beyond the Basics.

</img>I’m touring Europe in 2011. I have planned some public courses and some conferences, and am searching for private consulting and training work. Over the coming weeks I will reveal where I’m appearing on the map. The colors on the map provide a hint! If you’d like to invite me to your event, then please visit here. See you later this year!

I found someone trying to work on improving names in their code, and gave them a little advice. It gives a reasonable example of some of the things I think about when naming.

Emma López (@hell03610) shared with the world her notes from my XP2011 session, “A Simple Approach to Modular Design”. In it, she summarises a model I teach about how to improve names in code. I do plan to write about this in more detail this summer.

We’ve read this before: don’t reuse code by subclassing. Instead, compose objects into a loose network that share responsibility for the system’s behavior. While practising, working through Growing Object-Oriented Systems, I reached the point where Steve and Nat started writing microtests, and decided to stray from the text and try it out on my own. I reached the stage where the first two system tests pass, but in the process, I introduced much cyclic dependency and duplication, particularly related to sending and parsing chat messages. In preparing to address this problem, I noticed some duplication in how the auction handles its incoming chats: sometimes it updated the UI, and sometimes it didn’t. I figured I’d turn those into two chat message handlers, separating the UI behavior from the non-UI behavior, and in so doing, introduced some temporary duplication.

You can probably see a straightforward way to start removing this duplication. To illustrate it, look at the following code snippet.

So we have two classes with a common field, a common method signature, and similar method bodies. Extract Superclass seems logical, even though I can’t think of a decent name for this class so far. No worry: it will come to me.

Next, we have to extract the parts that differ from the parts the match each other. I did that by introducing new methods.

Next, we make processMessage() identical in both classes, so as to pull it up into FooMessageListener. And here, we find our first warning sign about this refactoring. The signatures of handleBiddingStateEvent() don’t match:

• BidsForSniperMessageListener.handleBiddingStateEvent(Chat, BiddingState)
• UpdatesMainWindowMessageListener.handleBiddingStateEvent(BiddingState)

In order to find out whether this will become a problem, I have to add Chat to the parameter list for UpdatesMainWindowMessageListener’s implementation of the method, at least for now. I imagine I could do something more clever, but I’m not sure that introducing a closure over a Chat object would simplify matters. I can put that on my “to try” list for now. In the meantime, I add the Chat parameter. See the diff.

Now to turn similar processMessage() implementations into identical ones, I introduced an empty method. This is another warning sign, since I can’t envision a “default” UI update. Still, I reserve judgment until I have more evidence, and introduce the necessary changes. See the diff.

Now that processMessage() looks identical in both subclasses, we can pull it up into FooMessageListener, for which we still don’t have a good name. Either see the diff or look at the final result below.

Now that I look at this last version of Main, I see the value in having attempted this refactoring. It clarifies quite well the specific reasons why I won’t let inheritance play a long-term role in this part of the design. Specifically, look at the little problems with this design.

• I still can’t think of a good name for FooMessageListener, although maybe you’re screaming one at me as you read. (Sorry; I can’t hear you.)
• BidsForSniperMessageListener.handleAllOtherEvents() doesn’t need to do anything, which might or might not be a problem, but often points to the Refused Bequest smell.
• UpdatesMainWindowMessageListener.handleBiddingStateEvent() doesn’t need its chat parameter.
• Both subclasses need a reference back to Main, but the superclass FooMessageListener doesn’t need it. Of course, that says more about Main in general than the FooMessageListener hierarchy: Main violates the Single Responsibility Principle in a variety of delightful ways. That’s why we’re here.

So let me talk this through: we have subclasses of FooMessageListener that respond to the same logical stimuli, but different subclasses need different data and sometimes a subclasses doesn’t need to respond to the stimuli at all. That sounds an awful lot like an event mechanism to me.

And if you look ahead in Steve and Nat’s book, that’s not surprising.

So how do we get from here to there? I plan to attack it this way:

1. Make FooMessageListener an event source for this new type of event, which I’ll tentatively call an AuctionEvent.
2. Turn the subclasses into AuctionEventListeners.
3. Inline stuff back into place.

You can follow the detailed changes on this branch starting with this commit.

By the end of this session, I ended up with a new Auction Event concept, which has started separating the “chat message” concept from the “auction changed” event. It does, however, lead to other questions, which I’ve highlighted with SMELL and REFACTOR comments, so look for them. For now, I feel good that, while introducing a hierarchy had its problems, it helped me see how specifically to eliminate it. I trusted the mechanics, and it helped me see where to go next. I know I need to introduce an “auction closed” event and perhaps need to introduce an AuctionEvent object to eliminate the need for type checking in AuctionEventSourceMessageListener. We’ll go there next time, but in the meantime, peruse the current state of the code.

If you prefer, look at the changes in moving from MessageListeners to AuctionEventListeners.

I hope I’ve shown how a code base can reject an attempt to reuse code by inheritance, prefering instead, in this case, composition implemented as an event chain.

</p>

I have noticed a recent swell of comments about the pain of dependency injection, and if this has caused you problems on your project, then I think I can help you by offering a few simple guidelines and a single goal for injecting dependencies.

The guidelines

I have formulated these as Novice or Advanced Beginner rules, and so I have worded them more strongly than I tend to word my advice.

• When in doubt, inject each dependency directly into the method that requires it.
• Only when you inject the same dependency into multiple methods of the same class, move the parameter into the constructor.
• Do not use so-called “setter injection” except as an intermediate step in a refactoring aimed at injecting the dependency into a method or the constructor.
• Stop using the Service Locator pattern, and instead inject the service into the client.
• Stop instantiating collaborating services (in the Domain-Driven Design sense), and instead inject those services into the client.
• When a constructor parameter list becomes uncomfortably long, split the class so that the new classes’ constructor parameter lists don’t overlap.
• If you notice a class using the same dependency through two different object graph routes, split the class so that the new classes receive the dependency directly in their constructors.
• If you notice a class using groups of dependencies at different times, split the class so that the new classes only use each cohesive group of dependencies.

All these guidelines have their roots in “remove duplication” and “fix bad names”, two of the four elements of simple design.

So why inject dependencies at all?

Some people inject dependencies in order to stub, mock or spy on a method for testing. I used to use that as a primary motivation, but over the years that motivation has evolved into something more widely useful. I inject dependencies for one reason: to move the things that change in the direction of the client and the things that don’t in the direction of the supplier. Or, if you prefer, “abstractions in code, details in data”. Or, if you prefer, to avoid abstractions depending on details. Any of those will do.

You don’t buy it?

No problem. I’ll write more articles in this space showing examples of each of the guidelines, and you’ll have plenty of opportunity to share your opinions about them.

</p>

On March 1, 2010 I changed the phrase “integration tests” to “integrated tests” in this article.

Integrated tests are a scam—a self-replicating virus that threatens to infect your code base, your project, and your team with endless pain and suffering.

Wait… what?

I mean it. I hate integrated tests. I hate them, and with a passion. Of course, I should clarify what I mean by integrated tests, because, like any term in software, we probably don’t agree on a meaning for it.

I use the term integrated test to mean any test whose result (pass or fail) depends on the correctness of the implementation of more than one piece of non-trivial behavior.

I, too, would prefer a more rigorous definition, but this one works well for most code bases most of the time. I have a simple point: I generally don’t want to rely on tests that might fail for a variety of reasons. Those tests create more problems than they solve.

You write integrated tests because you can’t write perfect unit tests. You know this problem: all your unit tests pass, but someone finds a defect anyway. Sometimes you can explain this by finding an obvious unit test you simply missed, but sometimes you can’t. In those cases, you decide you need to write an integrated test to make sure that all the production implementations you use in the broken code path now work correctly together.

So far, no big deal, but you’ll meet the monster as soon as you think this:

If we can find defects even when our tests pass 100%, and if I can only plug the hole with an integrated tests, then we’d better write integrated tests everywhere.

Bad idea. Really bad.

Why so bad? A little bit of simple arithmetic should help explain.

You have a medium-sized web application with around 20 pages, maybe 10 of which have forms. Each form has an average of 5 fields and the average field needs 3 tests to verify thoroughly. Your architecture has about 10 layers, including web presentation widgets, web presentation pages, abstract presentation, an HTTP bridge to your service API, controllers, transaction scripts, abstract data repositories, data repository implementations, SQL statement mapping, SQL execution, and application configuration. A typical request/response cycle creates a stack trace 30 frames deep, some of which you wrote, and some of which you’ve taken off the shelf from a wide variety of open source and commercial packages. How many tests do you need to test this application thoroughly?

At least 10,000. Maybe a million. One million.

Wie ist es möglich?! Consider 10 layers with 3 potential branch points at each layer. Number of code paths: 3¹⁰ > 59,000. How about 4 branch points per layer? 4¹⁰ > 1,000,000. How about 3 branch and 12 layers? 3¹² > 530,000.

Even if one of your 12 layers has a single code path, 3¹¹ > 177,000.

Even if your 10-layer application has only an average of 3.5 code paths per layer, 3.5¹⁰ > 275,000¹.

To simplify the arithmetic, suppose you need only 100,000 integrated tests to cover your application. Integrated tests typically touch the file system or a network connection, meaning that they run on average at a rate of no more than 50 tests per second. Your 100,000-test integrated test suite executes in 2000 seconds or 34 minutes. That means that you execute your entire test suite only when you feel ready to check in. Some teams let their continuous build execute those tests, and hope for the best, wasting valuable time when the build fails and they need to backtrack an hour.

How long do you need to write 100,000 tests? If it takes 10 minutes to write each test—that includes thinking time, time futzing around with the test to make it pass the first time, and time maintaining your test database, test web server, test application server, and so on—then you need 2,778 six-hour human-days (or pair-days if you program in pairs). That works out to 556 five-day human-weeks (or pair-weeks).

Even if I overestimate by a factor of five, you still need two full-time integrated test writers for a one-year project and a steady enough flow of work to keep them busy six hours per day and you can’t get any of it wrong, because you have no time to rewrite those tests.

No. You’ll have those integrated test writers writing production code by week eight.

Since you won’t write all those tests, you’ll write the tests you can. You’ll write the happy path tests and a few error cases. You won’t check all ten fields in a form. You won’t check what happens on February 29. You’ll jam in a database change rather than copy and paste the 70 tests you need to check it thoroughly. You’ll write around 50 tests per week, which translates to 2,500 tests in a one-year project. Not 100,000.

2.5% of the number you need to test your application thoroughly.

Even if you wrote the most important 2.5%, recognizing the nearly endless duplication in the full complement of tests, you’d cover somewhere between 10% and 80% of your code paths, and you’ll have no idea whether you got closer to 10% or 80% until your customers start pounding the first release.

Do you feel lucky? Well, do you?²

So you write your 2,500 integrated tests. Perhaps you even write 5,000 of them. When your customer finds a defect, how will you fix it? Yes: with another handful of integrated tests. The more integrated tests you write, the more of a false sense of security you feel. (Remember, you just increased your code path coverage from 5% to 5.01% with those ten integrated tests.) This false sense of security helps you feel good about releasing more undertested code to your customers, which means they find more defects, which you fix with yet more integrated tests. Over time your code path coverage decreases because the complexity of your code base grows more quickly than your capacity to write enough integrated tests to cover it.

…and you wonder why you spend 70% of your time with support calls?

Integrated tests are a scam. Unreliable, self-replicating time-wasters. They have to go.

¹ True: few code bases distribute their complexity to their layers uniformly. Suppose half your 12 layers have only two branch points—one normal path and one error path—while the others have 5 branch points. 2⁶·5⁶ = 1,000,000 and for 4 branch points 2⁶·4⁶ > 262,000. You can’t win this game.

² Aslak Hellesøy points to a way to take luck mostly out of the equation. His technique for choosing high-value tests will certainly help, but it stops short of testing your code thoroughly. I believe you can achieve truly thorough focused tests with similar cost to writing and maintaining integrated tests even using the pairwise test selection technique. (Thanks, Aslak, for your comment on April 12, 2009.)

Read more in this series

I grew up using EasyMock, but near the end of the first draft of JUnit Recipes, I wrote this:

“In the time between writing this essay and sending the book to be printed, a new dynamic proxy-based mock objects package has appeared on the scene, called jMock (www.jmock.org). It picks up where EasyMock left off, as the EasyMock project went through a temporary lull in activity, between October 2003 and May 2004. Being so new, we do not have any experience using it, and so we cannot say much about it, but it does look promising and bears a look. If you have used EasyMock, then it is worth experimenting with jMock to see the difference. You may find you prefer jMock’s approach to that of EasyMock.”

Since 2004 I have used JMock almost exclusively to drive my designs in Java. I even like the strange-looking JMock 2 syntax which, I know, puts me in the minority. In the last two years, other test double libraries have gained mindshare, among which Mockito has become quite prominent. While I can’t give you a feature-by-feature comparison, I can tell you this:

When I want to rescue legacy code, I reach for Mockito. When I want to design for new features, I reach for JMock.

Different central assumptions of JMock and Mockito make each one better at its respective task. By default, JMock assumes that a test double (a “mock”) expects clients not to invoke anything at any time. If you want to relax that assumption, then you have to add a stub. On the other hand, Mockito assumes that a test double (sadly, also a “mock”) allows clients to invoke anything at any time. If you want to strengthen that assumption, then you have to verify a method invocation. This makes all the difference.

When I work with legacy code, I mostly write learning tests to discover how different parts of that legacy code behaves. Usually legacy code has obscene and overwhelming levels of interdependency, and Mockito helps me manage that, by allowing me to worry about one crazy dependency at a time.

When I design for new features, I mostly write design tests that describe the new behavior I want to implement. With the nice green field of a new interface, I need JMock to encourage me to clarify the interaction I need. Whenever my production code attempts to use a collaborator, JMock effectively reminds me to ensure that I want that interaction. Most importantly, JMock stops me from introducing dependencies that I don’t need.

I really like Fred Brooks’ use of the terms essential complexity and accidental complexity. Briefly, a code base’s essential complexity reflects the complexity of the problem. Automating tax audits will result in high essential complexity. A code base’s accidental complexity reflects the complexity we programmers add because we don’t design simply. In short, if it isn’t essential complexity, then it’s accidental complexity. Our job as designers includes minimising accidental complexity.

Mockito helps me tolerate high accidental complexity while I work to reduce it.

JMock tries its best to stop me from introducing accidental complexity.

That explains why I use JMock when designing for new features and why I’ll recommend using Mockito for rescuing legacy code.

I came across this question on the testdrivendevelopment Yahoo! group.

Hi everyone,

I’d like some advice/opinions on how to test some existing code. It’s a web application using Spring and struts.

I have a class called the ProcessedFilesManager which contains a number of methods used by Struts Action classes. This manager communicates with five different DAOs to get the information that some of the Struts actions are interested in. Now, I want to test this manager class (ProcessedFilesManager). The way I’ve started doing it is stubbing up each of the five DAOs, however, this is proving to be quite painful. I didn’t want to use a mocking approach, nor did I want to use a DB solution like Hypersonic, but now I’m open to suggestions.

Seeing as there a number of approaches I could use, what do you think would be best for this situation?

It feels wrong to stub the DAOs because what if I’m introducing behaviour in there that differs from the actual DAOs? My tests will not be accurate.

Any advice/comments would be much appreciated.

Read the thread

I used to have this fear, and I do something now that has eliminated that fear.

When I stub a DAO method, I make an assumption about what that DAO method does. I used to be worried about making the wrong assumption, but now I have a contract test for the DAO interface that tests for the assumption I’m making in my Service test. The contract test gives me confidence that any implementation of the DAO method passes the same tests, so every implementation of that DAO method behaves the way I assume it does. Once I have this, I feel comfortable stubbing that DAO method that way in a Service test.

A contract test is a test for an interface. I describe contract tests in some detail in JUnit Recipes, recipe 2.6, although back then I called them “abstract test cases” because I hadn’t yet discovered the better name “contract test”. If you prefer, I’ve provided a diagram showing some contract tests for a typical DAO class.

Since classes inherit methods from their superclasses, the Hibernate Customer DAO Test will inherit the contract tests from its superclass, as will the JDBC Customer DAO Test. This means that each implementation has to pass not only its own tests (like testClosesSession() or testClosesResultSet()) but also the tests inherited from Customer DAO Contract Test Template. (I call it a “template” because it plays the role of template in the Template Method design pattern.) When you test-drive a new implementation of Customer DAO, simply make the new test extend the contract test template and you’ll automatically inherit its contract tests. This way, I have confidence that any implementation of Customer DAO behaves the way I’d expect any Customer DAO to behave.

Returning to our example, these contract tests give me confidence to stub the DAO when I test-drive the Service, and that confidence brings with it a happy side effect. I am confident that findAllWithPendingOrders() only returns customers with pending orders, so I don’t have to worry about that issue at all when I design the Service that reports all customers with pending orders. Now that I notice it, Report All Customers With Pending Orders Service is really just a Report on Customers Service that needs a Customer Filter, which could be a Pending Orders Customer Filter. I don’t think I would have felt comfortable with this level of generalization if I weren’t so confident in the way I’ve separated the responsibilities.

The next time you want to avoid stubbing a method because you’re worried you’ll make a wrong assumption about what the method does, try writing enough contract tests to give you the confidence you need. I think you’ll like the results.

One of my faithful readers at http://www.jbrains.ca told me that he couldn’t find an RSS feed icon in his Firefox address bar. I thought I could implement that fairly easily, so I agreed to do it. In the process, I wrote this spec:

Ruby follows the Principle of Least Surprise, so I tried this:

That doesn’t work, as I found no #with_attribute nor anything like it. After I dug a little, I found out that I should write this:

I don’t like this, because it checks two things at once: it looks for an “RSS tag” and checks the href attribute. That HTML implemented the “RSS tag” as attributes on <link> creates the confusion. The extremist in me calls this an integrated test, but I prefer not to go there today. Suffice it to say that when this check failed, I didn’t immediately know why, and I insist on immediately knowing why a check fails. When I’ve done this in the path with XPath assertions in XHTMLUnit or XMLUnit, I’ve resorted to writing duplicate checks that build on one another, so I tried that here:

Here I sacrificed duplication to improve the clarity of the assertions. I just now noticed that that contradicts my usual rule that removing duplication matters more than improving clarity. I dno’t know what to say about that just yet. Even if I remove the duplicate code, I still have the duplicate check, so extracting to a method won’t really do much here. I want #with_attribute!

Not deterred, I tried introducing a custom RSpec matcher, since I don’t know what benefit that would give me, but it would benefit me somehow. When I tried to do that, RSpec told me that it couldn’t understand response.should have_tag because it couldn’t find a has_tag? on the response. I didn’t like the looks of the stack trace; I felt I would have to delve deeply into RSpec or assert_select, and I didn’t find myself in the mood to do either, so I switched to Nokogiri.

Here I got to write the spec the way I wanted to: assume you will find “RSS feed tag”, then check that its href attribute matches the right URL. If the response has no “RSS feed tag” at all, then complain violently, because of the higher severity of the mistake.

Of course, if you have another suggestion, I’d like to see it. Add your comment below.

Special thanks to Frivolog for helping me get the original have_tag code working.