Corey Haines interviewed me at the Golden Tulip Times Hotel in Bucuresti, Romania on the topic of Primitive Obsession: the dangers of using simple data types to represent complex information.

Meta: I need your opinion about reading this blog with RSS

Dear Readers, as you know, I post a lot of code samples in these articles and I use gists to do it. When I embed code samples in these articles, I use Javascript to do it, and almost certainly your RSS feed reader doesn’t render the code samples correctly. I have proposed a solution, but I want your feedback.

I have edited RSpec, have_tag(), Spec::Matcher, and Nokogiri to include links to the code samples for those browsers that have disabled Javascript. I have to duplicate a lot of code to make that happen, which I want to eliminate, but first I wanted to know how much you liked this, so please help me perfect the way I display code samples to browsers without Javascript.

Add a comment with what you like about my solution, and how you might improve it. Either subscribe by a standard RSS feed reader or disable Javascript in your browser (at least for thecodewhisperer.com, and of course, only temporarily), read the article I’ve edited and tell me if you have a better idea. Please, if you suggest something, then include a link to how to do it. I prefer not to break new ground here.

Thank you for your help.

Stop injecting dependencies... wait, what?

I apologize for ever suggesting that you should inject dependencies. I got that wrong. Don’t inject dependencies because some old, fat guy with a beard told you to. Inject a dependency as a step towards inverting that dependency. If you don’t plan to do the latter, then don’t bother doing the former.

RSpec, have_tag(), Spec::Matcher, and Nokogiri

This entry contains code samples that you can only view with Javascript enabled. Please read this in a browser with Javascript enabled while I figure out how to remove this restriction gracefully with Tumblr.

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: View code sample

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

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: View code sample

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: View code sample

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. View code sample

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.

Improving clarity by removing temporal coupling

I decided to try Neo4j, so I re-opened the code base I use for my old Test-Driven J2EE tutorial, since I don’t have any other Java project that uses any form of database. The Coffee Shop example I’ve used the past several years, while it has no SQL database implementation, has a Repository for customer registrations implemented in memory, by a fake, and with XML file-based persistence. It seemed a reasonable candidate to explore Neo4j, and implementing a Neo4j-based repository should take little effort.

Of course, when I read the code, things changed.

I hadn’t remembered the last time I’d looked at code I wrote and thought, What idiot designed this? I experienced that in full bloom when I looked for the tests for the registration repository. I didn’t know whether I had contract tests for the repository implementation, and I did, but what I saw there made me cringe.

It appears that createRepository() expects subclasses to invoke getRegistrationsToPrepopulate() even though it has no way to enforce that. That means that each subclass of FullRepositoryContractTestTemplate has to know that it must invoke getRegistrationsToPrepopulate() at the right time. I flashed back to when I first read about “programming by accident” in The Pragmatic Programmer. Even though I knew about programming by accident when I wrote this code in 2005, that didn’t stop me from creating a textbook example of it. I decided to look at the subclasses. The first one seemed innocuous enough.

This appears to capture the intended reuse. Not so bad. I looked at the next subclass. I couldn’t believe my eyes.

This one hurts. I feel shame. I see that createRepository() depends on setUp() running first, so that the test can initialize repositoryFile, which it does by invoking setupRepositoryOnDisk(), but only before invoking super.setUp(). Frankly, I don’t know how this ever worked, because the slightest breeze would knock it over. Fortunately, I can easily fix the accidental temporal coupling by replacing createRepository() with createRepositoryWith(registrations). Like any implementation change, I proceed in the Three Safe Steps:

1. Add the new implementation.
2. Migrate clients from the old implementation to the new implementation, one by one, slowly making the old implementation obsolete.
3. Remove the old implementation.

In this case, because I need to change a class in a way that affects all its subclasses, I change the design in each subclass of FullRepositoryContractTestTemplate first, starting with FullXstreamRepositoryTest. Here, I use the “New York, New York” heuristic: if I can make it there, then I can make it anywhere.

The tests pass, so I move to FullInMemoryRepositoryTest and change it.

Now I remove the duplication between the two subclasses by pulling the common method createRepository() up.

Now I don’t think createRepository() pulls its own weight any more, so I inline it.

I feel the same way about getRegistrationsToPrepopulate(), especially since “prepopulate” implies temporal coupling and I’ve successfully removed that now. I don’t need any more reasons to inline this method.

Now I turn my attention back on FullXstreamRepositoryTest, where I had duplicated code to implement createRepositoryWith(registrations). Now that I’ve removed the temporal coupling, setUp() no longer needs to invoke setupRepositoryOnDisk(). I verify that only setUp() invokes setupRepositoryOnDisk(), which allows me to remove the latter.

Next I look at the subtler temporal coupling: having to initialize repositoryFile before letting the contract test template invoke createRepositoryWith(registrations) by invoking setUp(). How did I ever get that right? I remove the indirection by assigning repositoryFile directly in createRepositoryWith(registrations), then removing setUp().

With all this done, I tried to test-drive the Neo4j implementation of Repository, but I realized that Neo4j didn’t fit the problem domain well, so I abandoned ship. On the bright side, when it comes time to implement Repository for a relational database, I will find it easier to do than I would have before these refactorings. I’ll leave that for another day.

WhyITakeTheTimeToNameThingsPreciselyEvenThoughThoseNamesCanGrowQuiteLongAtTimes

I recently coached a few teams in modular design and test-driven development and, in the process, made a small change to the way I explain the Four Elements of Simple Design. Specifically, this concerns the point where I suggest that people focus on removing duplication and fixing bad names.

When I give people long-range, culture-change, you-have-to-figure-it-out-on-your-own advice, I believe I also owe them some follow-these-rules, you-can-start-on-Monday advice. While I believe most people easily get started removing duplication, fewer understand how to start fixing bad names, because to fix them, you need to know specifically what to fix and specifically how to fix it. I wanted to give these people some practical advice on improving clarity in their code base by improving names. I settled on this advice: make names more precise.

Before I continue, I leave it to you to investigate the difference between “accurate” and “precise” since, even today, many people confuse the two. If you can’t rattle off the difference between these two words in the next ten seconds, then please look them up. Thanks.

Making names more precise, whether for variables, methods or classes, helps me identify emerging structures in my design. Patterns in names call my attention to smaller structures trying to burst out of larger ones. I can give you a few examples of the patterns I see.

• While extracting methods, I see many private methods with the same word or phrase in the names
• I extract a method whose name matches, at least in part, the name of one of its parameters
• I extract shorter methods from a long method and see that the long method invokes the shorter methods in recurring groups by their names
• I extract complex expressions into variables and find similarities in the names

Each of these merits an article on its own, so I will defer the details until then. In each case, naming things precisely helps me see the emerging pattern and even helps me name the emerging element, whether new methods, interfaces, or classes. Vague names simply don’t do that.

Please reflect on this question: why do you name things vaguely? What tempts you to name something “compute” or “execute” or “do” or “go”? Why not simply name things as precisely as you can?

I’ve seen and heard a variety of answers, ranging from I don’t want to type that much to Names shouldn’t run 60 characters long to I really don’t know. I conjecture that people pay less attention to names because they don’t see a concrete benefit to making names more precise.

Designers talk passionately, but vaguely, about so-called maintainability or readability without articulating what those words mean. Those designers might know for themselves what they mean, but unless they can explain that meaning to others, they rely on each person’s innate pride in work to name things well. Sadly, hope in the innate pride of workmanship in others doesn’t scale to great designs in large systems. Let me, therefore, articulate a concrete benefit of naming things precisely.

When I name things precisely, I more easily see when similar things are far apart and different things are close together, both signs of poor cohesion. When I name things precisely, I more easily see duplication in the intent of blocks of code, and can more easily remove that duplication, resulting in fewer potential defects. When I name things precisely, I more easily see patterns in names of private methods that suggest too many responsibilities in one place, and when I split those responsibilities, I decrease the coupling in a system. I know that I should fix problems related to low cohesion, high defects and high coupling, and naming things precisely helps me find those problems more easily.

I hope these guidelines help you in your designs.

What your tests don't need to know will hurt you

I just finished reading Brian Marick’s article, “Mocks, the removal of test detail, and dynamically-typed languages”, which focused me on a design technique I use heavily: awareness of irrelevant details in tests. Referring back to the four elements of simple design, I focus on irrelevant details in tests in order to help me maximize clarity in production code. Please allow me to sketch the ideas here.

I use the term irrelevant detail to refer to any detail that does not contribute directly to the correctness of the behavior I’ve chosen to check. I know when I’ve bumped into an irrelevant detail: while writing the code that allows me to check something, I start typing something, then my shoulders slump and I exhale with annoyance. I think, I shouldn’t have to write this, because it has nothing to do with what I want to check. Brian’s example illustrates this perfectly:

The random methods save a good deal of setup by defaulting unmentioned parameters and by hiding the fact that Reservations have_many Groups, Groups have_many Uses, and each Use has an Animal and a Procedure. But they still distract the eye with irrelevant information. For example, the controller method we’re writing really cares nothing for the existence of Reservations or Procedures–but the test has to mention them.

I maintain one Ruby code base, which runs my weblog at jbrains.ca and I often find myself creating a Posting object in my tests, and often the content of that posting doesn’t matter.

I use a simple rule to help me identify irrelevant data in tests.

If I can change a value without changing the result of the behavior I want to check, then I call that irrelevant data for this test.

I first identify data as irrelevant and mark it that way. For string values, I include the word “irrelevant” in the string. Some people use words like “dummy” for this purpose, but I prefer “irrelevant”, because I don’t want others to confuse an irrelevant detail for a type of stub. I used to simply choose random values for irrelevant data, because I had read that good testing principles included varying data from test to test. Now I feel that choosing especially meaningful-looking values for irrelevant data obscures the purpose of a test.

Irrelevant data can hurt in a number of ways.

• You might “play it safe” and duplicate the irrelevant data in more tests, leading to excess investment in maintaining your tests.
• You might “play if safe” and check the irrelevant data in your test, leading to misleading failures as you change unrelated behavior in the system.
• Changing the data might affect the result (pass/fail) of your test even though the data does not relate conceptually to the behavior you want to check.

I estimate that I have experienced more pain from this last effect than from all other effects combined.

Once I have identified and labeled irrelevant data, I look for ways to eliminate it. Moving irrelevant data into fixtures or ObjectMothers, hides the symptoms without solving the problem. Ironically, moving irrelevant data into fixtures or ObjectMothers merely makes that data easier to spread to more tests to which they do not relate, creating more, not less, potential for unhealthy dependency. It masks the very kind of observation that Brian made in his code base. Sometimes, I write a test and notice that not both the value and the type of a piece of data doesn’t matter. I find this happens a lot when I test-drive controller behavior that takes data from a model and hands it directly to a view.

I notice here that [1,2,3] represents “any non-empty array”. While writing this sentence, I wondered if I could change this to Object.new, so I tried that, and the test passed. In this case, while the actual data and type don’t matter, I need the model to return anything but nil to ensure that the view has something and that that something came from the model. With this realization, I rewrote the test.

Instead of should == I use should equal() here, which translates to assertSame() in other languages, to emphasize that I expect the controller to take whatever it receives from the model and hand it to the view. When I want to check what the view does with it, I’ll send valid data to the view and check it in isolation. When I want to check what the model returns, I’ll look at what the view expects and check that the model can provide it. The controller need not bother itself with the details.

Compare this with the corresponding integration test for checking the controller, which would require knowing all these otherwise irrelevant things and setting all this otherwise irrelevant data…

• How to create a Posting in the database in the “queued for publication” state, which means setting the queued_at attribute to a time in the past, but the published_at attribute to nil.
• How to instantiate a valid Posting, which requires title and content.
• Which view the controller renders when it displays the publication queue.
• Which attributes of the Posting the view expects, to ensure that I populate them with valid, if meaningless, data.

I suppose I could think of more irrelevant behavior and data, but this will do. How does this irrelevant data and behavior hurt specifically in this situation?

• I have this note in my problems list: “A Posting can be both queued and published. I want to remove this possibility, rather than forcing Posting to maintain the invariant.” When I fix this problem, my controller test will change, even though I won’t have changed the controller.
• If I added a mandatory attribute to Posting, then my controller test would change, even though I wouldn’t have changed the controller.
• If I removed a mandatory attribute from Posting, then my controller test would have even more irrelevant data, meaning more accidental ways to go wrong.
• If I changed the view that the :new action renders, then my controller test would change, even though I wouldn’t have changed the controller behavior that my test checks.
• If I changed which attributes of Posting the view processed, then my controller test would either need to change or contain even more irrelevant details than it did before.

When I write that integration tests are a scam, I include as a reason that writing integration tests encourages including irrelevant details in tests, and by now I hope I’ve shown some ways that that hurts.All this leaves me with a few guiding principles to use when writing tests.

A test should make it clear how its expected output relates to its input.

I can use this guiding principle to develop some Novice rules:

Mark all irrelevant data in a test by extracting the values to variables whose name includes the word “irrelevant”.Hide all irrelevant data using techniques like the ObjectMother pattern. Remember to remove duplication after hiding irrelevant data.Call attention to all input values that have a direct bearing on computing the expected result in a test.

Beyond these novice rules, this guiding principle helps in two key ways. Certainly, it encourages me to write shorter, more focused tests, which tend to have all the properties I want in a test, but more importantly, it leads me to a higher guiding principle when writing tests.

When you find it difficult to write a concise, focused, isolated test, the production code has unhealthy dependencies to loosen, invert, or break.

Here, “concise” means having no irrelevant details; “focused” means failing for only one reason; “isolated” means executing without side effects on other tests.

I loosen dependencies by applying the Generalize Declared Type refactoring. I most commonly invert dependencies by extracting an interface and using constructor injection. I most commonly break dependencies by either extracting a memento or the return value of a method and depending on that, rather than the original object.

This guiding principle leads me in the direction of The Fundamental Theory of Test-Driven Development.

Assuming we know what we want to check and understand the mechanics of how to check it, difficulty checking behavior means unhealthy dependencies in the design.

A response to "Manual Mocking: Resisting the Invasion of Dots and Parentheses"

The twittersphere has been all abuzz today because of something I tweeted early this morning (follow @unclebobmartin). In my tweet I said that I hand-roll most of my own mock objects in Java, rather than using a mocking framework like mockito.

The replies were numerous and vociferous. Dave Astels poignantly stated that hand-rolling mocks is so 2001!

So why do I roll my own mocks?

So begins the article in which Bob Martin describes why he prefers to hand-roll his test doubles, which he annoyingly insists on calling “mocks”, rather than use a library (not a framework!) like JMock or Mockito. I would like to respond here.

Sorry, Bob, but I can just as easily explain the problem with your central example in a way that doesn’t involve the question of hand-rolled v. frameworky mocks.

You stub GSSName to return a stub GSSCredentials. I smell a foul dependency right there. What exactly are you checking here?

Next, you verify (I guess this is an expectation) that NegotiateAuthenticator invokes manager.createName() with the expected parameters, but then you check those parameters. Are you checking that NegotiateAuthenticator correctly computes the serviceNameType and mechanism? If so, then why not stop there? Moreover, why does NegotiateAuthenticator need to talk to a manager in order to check data coming from the properties? I smell a coupling problem, and specifically an SRP violation.

Finally, you check that NegotiateAuthenticator asks the manager to try to create a credential with the right parameters then check that it stores the credential that created. Why check so many things at once?

I admit that I can’t check that my new tests compile, but I wrote these ones based on what I could understand of what you’ve done here.

When I tried to do this, I realized that you are essentially using a stub to do something a mock could do. Specifically, you are using authenticator.getServerCredentials() to check, quite indirectly, that NegotiateAuthenticator asks GSSManager to create server credentials with the proper parameters. I imagine you don’t care (in this test) that the authenticator’s serverCredentials are non-null, but rather that the authenticator creates the right serverCredentials. In that case, once you verified your invocation of manager.createCredential(), you finished your job!

If you also want to check that authenticator.getServerCredentials() answers whatever credentials it asked the GSSManager to create, then you can write a much simpler check for that, which I’ve provided as produceServerCredentialsOnDemand().

Although I have to admit, Bob, that I still don’t quite understand what you’ve tried to check here. I suppose I’d need to pair with you on this.

Checking a method directly or indirectly

I received this question by email.

Think about the code below. If we have a method named doingSomething() and we want to test some behavior of it. In execution of doingSomething() we make a call to other method doingOtherThing(). How can we test doingSomething() method isolated from doingOtherThing() method? I can not do this with isolation from doingOtherThing() method. I just tested doingOtherThing() and if it passes test doingSomething(). Is this valid for unit testing? In unit testing what is our unit? A class or a method itself? Thanks for your patience.

I would like to ask the questioner a number of questions of my own, but first I’d like to offer some advice and hope I don’t make too many mistakes in the process.

How can we test doingSomething() method isolated from doingOtherThing() method?

I interpret this question to mean “How do I test doingSomething() without invoking doingOtherThing()?” and answer that question first.

What I don’t like about this design

The correctness of doingSomething() depends on the correctness of doingOtherThing(), so I can’t check doingSomething() without also checking doingOtherThing(). The questioner noticed the same concern. My Blink reaction says A has two responsibilities that I should move further apart. I can see this as either a violation of the Single Responsibility Principle or as a violation of the cohesion principle.

My preferred solution

If I felt comfortable changing all the code, then I would introduce an interface OtherThing with method doingOtherThing(), and introduce a new method doingSomethingWith(OtherThing) that does what doingSomething() does now. This results in a design in which I’d test domethingSomethingWith() and let doingSomething() be one line of untested code. It would look like this:

How to implement my preferred solution safely

Always run the tests after every step.

1. Extract a new interface OtherThing from class A, putting method doingOtherThing() on the new interface.
2. Copy the method doingSomething() as a new method doingSomethingWith(OtherThing otherThing), changing doingOtherThing() to otherThing.doingOtherThing().
3. Change method doingSomething() to the one line of code invoking doingSomethingWith(this), because this implements OtherThing.

Next steps

I would certainly consider removing the one line of untested code, meaning the method doingSomething() invoking doingSomethingWith(this). I could simply inline doingSomething(), but I would usually change A to accept an instance of OtherThing in its constructor. This design would result.

I might prefer to wait until another method on A wanted to use OtherThing before taking this step, especially if I knew very little about the design or felt particularly pressured to finish quickly.

Now let me ask the other direct question.

Is [testing doingOtherThing() by testing doingSomething()] valid for unit testing?

In short, no, but I want you to know the longer answer.

I advise Novice practitioners to write any test they know how to test, even if the test is bigger than needed, longer than needed, or more complex than needed. I recommend Novice practitioners spend as much time practising writing tests as they can.

I advise Advanced Beginners to try testing doingSomething() in isolation by using the Subclass To Test pattern from Working Effectively with Legacy Code on class A.

I advise Competent practitioners to try separating the doingOtherThing() behavior from A by extracting the interface as I did in this article. I suggest they notice the duplication between the tests for doingOtherThing() and doingSomething() to motivate the refactoring.

I advise Proficient practitioners to weigh the options of extracting the interface from A against testing doingOtherThing() indirectly and living with the duplication in the correspond tests.

I’d ask an Expert practitioner whether he thinks we should decouple the two behaviors in A or change A in such a way to make those two behaviors closer to each other. (It always surprises me when we can do this!)

In unit testing what is our unit? A class or a method itself?

I advise Novices to treat the class as a unit.

I advise Advanced Beginners and Competent practitioners to treat the method as a unit.

I advise Proficient practitioners to treat the “interesting behavior” as a unit, and recognize that sometimes we don’t implement “interesting behavior” as a single method. Consider the “add()/contains()” problem as an example of an “interesting behavior” that requires two methods.

Part 4: Mark Needham and his book club on "Integration Tests Are A Scam: The Movie"

Mark Needham recently wrote that his book club has discussed my presentation Integration Tests Are A Scam and had a few comments and questions. I wanted to address those questions, finishing with the last two.

I think we still need some integration tests that go through the user interface particularly if we are writing javascript heavy front ends–from my experience only using unit tests doesn’t take us the whole way to having confidence that this type of code works. –Mark Needham

I won’t claim to have extensive experience here, so I can only speak about what I’d try in that situation. If I have to write a Javascript-heavy user interface, then I’d want to test-drive the technology view very carefully, and certainly separately from the rest of the application. I would probably test-drive the technology view separately even from the logical view, either with JSSpec or by generating the Javascript from another language. Google Web Toolkit lets me do this from Java, and Ruby has its Javascript generators. I simply wouldn’t see the need for integration tests here.

I like the idea that acceptance tests are supposed to be clarifying requirements and not testing correctness although it seems to be really easy to cross the line where they do end up verifying the correctness of our application.

Thank you. If I can help business and technical people come together to use examples to clarify features, then I think I’ve done my job. I don’t want to see business people put their specifications into tables then throw them across the room (or the server) to the programmers to consume. I just don’t see much value in that.