Red/Green/Refactor, For The Right Reasons

First, let me say this: WHAAaa???? something useful occurred on Google Buzz?! :) Ok, now that I’ve got that over with… there was a useful stream of comments over on the extended twitter reply network from one of my tweets a while back.

 Jeff Doolittle - that applies to all three! red (for the right reason), green (for the right reason), refactor (for the right reason)

Jeff’s comment at the bottom is what I’m referring too as the useful bit, really. I often catch myself going through the motions of skip-right-to-green/refactor, or catch myself doing ignore-the-reason-why-it-failed/green/move-on-to-the-next-thing. Both of these test-first anti-patterns have bitten me in the butt on multiple occasions. I’d guess that they actually bite me pretty regularly, if not every time I fall into one of these anti-patterns.

Jeff is right, and I know it from experience and personal pain… it’s not just red/green/refactor. It’s actually

Red: for the right reasons

Green: for the right reasons

Refactor: for the right reasons

What is the right reason? Well, that depends entirely on your circumstances and the code that you are writing and testing.

 

Red: For The Right Reasons

Red should happen because the conditions of the test were not matched… not for any other reason. As one example: this means that if you are expecting an interface method to be called, the test should fail because the method was not called. It should not fail for any other reason.

An null reference exception is usually not the right reason. There are times when checking null or not-null is the right thing to do, but you should not be getting exceptions to tell you whether its null or not… most of the time, anyways. There’s some exceptions to this, of course.

A “Throw NotImplementedException();” is never the right reason. No, I do not want to throw an exception from that method that I just generated. I want it to be empty, or return a null or some other default value if there is a return type. (and yes, i know there is an option for this in Resharper… but i’ve never seen that option work, no matter how many times i’ve tried to make it work).

There are plenty of other bad reasons for red, and each specific test has to be evaluated to understand the right reason for red. Context truly is king.

 

Green: For The Right Reasons

A test should pass not just because the conditions stated in the assertions exist, but because the simplest thing that could possibly meet the requirements was implemented.

Back when I was doing all state based testing, I would often make my test go “green” by hard coding a value into it. For example, if I was asserting that the output of some calculation was supposed to be 60, I would hard code a “return 60;” in the method that was being called. I did this to “calibrate” the test… to make sure that the test could pass for the right reasons, even if the implementation was not yet correct. I’m not sure I buy this anymore, really. Do I really need to know that “Assert.AreEqual(60, myOutputVariable);” will work because I hard coded a value of 60 into the method being called? I really don’t think so.

The simplest thing that could possibly work is a misnomer. Hard coding a value is the simplest thing, but it’s worthless. Setting up the very basics of structure and process, without regard for any not-yet-coded functionality is a much more appropriate way to look at the simplest thing. You need to account for the actual requirements of what you are implementing, the core of the architecture that your team has standardized on, and any other bits and pieces of standards that make sense and should be in place for the system in question.

Did you get your test to pass? Good. Did it pass because you wrote production worthy code, without writing more than would make the test pass? Great!

 

Refactor: For The Right Reasons

STOP! Hold on a second… before you go any further and before you even think about refactoring what you just wrote to make your test pass, you need to understand something: if your done with your requirements after making the test green, you are not required to refactor the code. I know… I’m speaking heresy, here. Toss me to the wolves, I’ve gone over to the dark side! Seriously, though… if your test is passing for the right reasons, and you do not need to write any test or any more code for you class at this point, what value does refactoring add?

Just because you wrote some code to make a test pass, doesn’t mean you need to refactor it. There is no rule or law that says you must refactor your code after making it green. There is a principle that says you should leave the campsite cleaner than when you got there. Sometimes the process of adding code and functionality leaves it clean to begin with and you just don’t need to do any additional work to clean it up.

If you write your first chunk of code to make a test pass and it’s simple, elegant and easy to understand and modify, then stop. You’re done. You don’t need to refactor. If you are adding the 25th chunk of code based on the 25th test and the code remains simple, elegant and easy to understand and modify, then stop! You don’t need to refactor. If, however, you write some code that is a duplication or is using some nested if-thens and loops, or is using hard coded values, or for whatever other reason it does not conform to all of the principles and guidelines that you follow in order to make your code simple, elegant, easy to understand and modify… ok, then you need to refactor.

So why should you follow the refactor portion of red/green/refactor? When you have added code that makes the system less readable, less understandable, less expressive of the domain or concern’s intentions, less architecturally sound, less DRY, etc, then you should refactor it.

 

In Retrospect

Looking back at my tweet in the image at the top of this post, it’s obvious that I was painting an oversimplification of red/green/refactor. This mantra was never intended to be a simple mechanical checklist of things that you must do in order to be a TDD’er. The entire purpose of the mantra is to remind us that we need to not only write tests as specifications to prove that our system works, but also consider the changes that we are making to the system in the context of clarity and expressiveness of code.

There is no simple answer for “what is the right reason?” when considering red/green/refactor. Each situation has to be evaluated based on its needs and its context. The examples that I’ve given for good and bad reasons are only examples to try and illustrate the principles behind the right and wrong reasons.

In the end, red/green/refactor is just a convenient statement that should remind us to think about what we are doing and apply pragmatism to our process by ensuring we are adding value, not just blindly following some simple platitude without regard for the consequences.


Post Footer automatically generated by Add Post Footer Plugin for wordpress.

About Derick Bailey

Derick Bailey is an entrepreneur, problem solver (and creator? :P ), software developer, screecaster, writer, blogger, speaker and technology leader in central Texas (north of Austin). He runs SignalLeaf.com - the amazingly awesome podcast audio hosting service that everyone should be using, and WatchMeCode.net where he throws down the JavaScript gauntlets to get you up to speed. He has been a professional software developer since the late 90's, and has been writing code since the late 80's. Find me on twitter: @derickbailey, @mutedsolutions, @backbonejsclass Find me on the web: SignalLeaf, WatchMeCode, Kendo UI blog, MarionetteJS, My Github profile, On Google+.
This entry was posted in Agile, Analysis and Design, AntiPatterns, Pragmatism, Principles and Patterns, Refactoring, Unit Testing. Bookmark the permalink. Follow any comments here with the RSS feed for this post.
  • http://nolanegly@edgewolf.com Nolan Egly

    There are times I find it useful to first implement by returning a hard coded value to make sure the setup of the test works, and I don’t feel this is worthless. Usually this is when spinning up an object and then interacting with it to move it into a specific state before executing the test.

    For example…
    Foo foo = new Foo();
    foo.Initialize(someStateValue); // this might cause the test to fail if wrong state value is used
    int result = foo.StatefulBehaviorUnderTest();
    result.ShouldBe(60);

    If foo.Initialize() is sufficiently complex, or should have been called with someOtherStateDependentValue, doing a hardcoded return is the easiest way to verify the test fails ONLY because the result isn’t correct, and not because the initialization is wrong. In other words, if the hardcoded implementation doesn’t make the test go green, the test is failing for the wrong reason.

    I completely agree with your comments on applying critical thinking, and not just operating under blanket principles.

  • http://www.lostechies.com/members/derick.bailey/default.aspx derick.bailey

    @Nolan,

    i’m not sure i follow you, or maybe you didn’t understand what i meant… are you saying the “foo.Initialize(someStateValue);” is the hard coded value? no… i do that a lot to set up the state of the object. what i meant was “foo.StatefulBehaviurUnderTest();” would have a hard coded “return 60;” in it…. that, i don’t agree with anymore.

    or am i misunderstanding what you’re saying?

  • http://nolanegly.edgewolf.com Nolan Egly

    In production, the value that Foo.StatefulBehaviorUnderTest() is supposed to return depends on how Foo.Initialize() is called.

    The test is verifying Foo.StatefulBehaviorUnderTest() returns the correct result when Foo is in someStateValue.

    In order to verify the test fails SOLELY because Foo.StatefulBehaviorUnderTest() is incorrect, you must be certain that Foo.Initialize(someCertainState) is not causing your test to fail. Sometimes the only way to ascertain this is to hardcode the result. If the test fails, then it’s an incorrect test setup, not the implementation. Let me try recommenting the simplistic example:

    Foo foo = new Foo();
    // this might cause the test to fail, maybe it throws something
    foo.Initialize(someStateValue);
    // this is hardcoded to return 60
    int result = foo.StatefulBehaviorUnderTest();
    // if this fails, foo.Initialize() is breaking the test somehow
    result.ShouldBe(60);

    The example is overly simplistic, but illustrates what I’m trying to say: sometimes making sure the test fails for the right reason means needing to verify the setup of the object under test isn’t the cause of failure, and hardcoded results is a quick way to check that.

  • http://www.lostechies.com/members/derick.bailey/default.aspx derick.bailey

    @nolan,

    “sometimes making sure the test fails for the right reason means needing to verify the setup of the object under test isn’t the cause of failure, and hardcoded results is a quick way to check that.”

    ah – ok. that’s the key. i had “green for the right reason” stuck in my head, and you were talking about “red for the right reason”…. 100% agree with you on this one, with read for the right reason. i do that fairly regularly to prove the test can fail for the right reason.

  • http://twitter.com/pseale Peter

    Mark Needham posted a good defense of faking the implementation instead of implementing the “Obvious Implementation” – http://www.markhneedham.com/blog/2009/05/24/tdd-making-the-test-green-quickly/ – summary: both approaches are valuable.

  • http://www.twitter.com/adambelshaw Adam B

    Terrible grammar but very good post. Cheers Derick!

  • http://www.thomas-weller.de Thomas Weller

    Hi Derick,

    “A “Throw NotImplementedException();” is never the right reason” for a test to be red !? I’m not sure I’m buying that. I don’t exactly understand the problem with that approach.

    Having a test being red because of a NotImplementedException tells me that my test set up is right and the test fails only because the method under test is not yet implemented – and that’s exactly what I want. The effect is the same that you get when hard-coding a return value, only that you don’t depend on a specific value and you get the code for free with R#. – Or, in your words: If the test fails because of a NotImplementedException, it is red for the right reasons (at least in my eyes…).

    I use this approach regularly in TDD and I made very good experiences with it – especially because with a NotImplementedException a test will NEVER become green for the wrong reasons, and therefore there will be no danger that it is overlooked in the (sometimes hectic) production process.

    This is no religion, of course. But I don’t really get your arguments at this point (while I’m totally d’accord with the rest of your post). Maybe you could clarify a bit more on that…

    Regards
    Thomas

  • http://www.lostechies.com/members/derick.bailey/default.aspx derick.bailey

    @Thomas,

    i’ve been thinking about this for a few days now, and am trying to get my thoughts organized. i might reply directly here or in a new blog post, depending on how much stuff i have to say.

    thanks for the good question, by the way. i like it when people challenge the things i say and ask me to explain why. forces me to stop and think through what i’ve said and why i think that way.

  • http://www.lostechies.com/members/derick.bailey/default.aspx derick.bailey

    @Thomas,

    my full response with example and explanation is available in my “Red For The Right Reasons: Fail By Assertion, Not By Anything Else” blog post. The title sums it all up, though. Hope that idea and that post help illuminate why I don’t like not implemented exceptions.

  • http://www.agilification.com Jeff Doolittle

    @Derick – I’m finally catching up on my backlog in Google Reader and I just now found this! Heh.

  • Michael Baun

    Refactoring for the right reasons.

    “TDD = Refactoring + TFD. ”
    “TDD completely turns traditional development around. When you first go to implement a new feature, the first question that you ask is whether the existing design is the best design possible that enables you to implement that functionality. If so, you proceed via a TFD approach. If not, you refactor it locally to change the portion of the design affected by the new feature, enabling you to add that feature as easy as possible. As a result you will always be improving the quality of your design, thereby making it easier to work with in the future.”

    http://www.agiledata.org/essays/tdd.html

    So refactoring should only be done at the point it makes a new feature easier to implement – if you need to refactor your implementation you should immediately look back and realize that you refactored the architecture wrong. So the first red green stuff should test the refactoring – not the feature implementation. So there are at least two fundamental reasons tests are written. To test the refactoring – and to test the feature. The refactoring is not the feature – it is the groundwork for it. It is the developing architecture.

    Anyway, hope this is not too off topic. I am new to TDD – as this post probably makes apparent but isn’t this really to the point of what TDD is for – architectural flexibility.

    I am refereing to the statement.
    “A test should pass not just because the conditions stated in the assertions exist, but because the simplest thing that could possibly meet the requirements was implemented.”.

    So here is my loaded question is architecture a requirement?

    I am new at this am just trying to understand where all this is coming from.

    Anyway, off to enjoy the rain.