13

What do you do when you are writing a test and you get to the point where you need to make the test pass and you realize that you need an additional piece of functionality that should be separated into its own function? That new function needs to be tested as well, but the TDD cycle says to Make a test fail, make it pass then refactor. If I am on the step where I am trying to make my test pass I'm not supposed to go off and start another failing test to test the new functionality that I need to implement.

For example, I am writing a point class that has a function WillCollideWith(LineSegment):

public class Point {
    // Point data and constructor ...

    public bool CollidesWithLine(LineSegment lineSegment) {
        Vector PointEndOfMovement = new Vector(Position.X + Velocity.X,
                                               Position.Y + Velocity.Y);
        LineSegment pointPath = new LineSegment(Position, PointEndOfMovement);
        if (lineSegment.Intersects(pointPath)) return true;
        return false;
    }
}

I was writing a test for CollidesWithLine when I realized that I would need a LineSegment.Intersects(LineSegment) function. But, should I just stop what I am doing on my test cycle to go create this new functionality? That seems to break the "Red, Green, Refactor" principle.

Should I just write the code that detects that lineSegments Intersect inside of the CollidesWithLine function and refactor it after it is working? That would work in this case since I can access the data from LineSegment, but what about in cases where that kind of data is private?

4 Answers 4

14

Just comment out your test and recent code (or put into a stash) so you have in fact turned back the clock to the start of the cycle. Then start with the LineSegment.Intersects(LineSegment) test/code/refactor. When that's done, uncomment your previous test/code (or pull from stash) and keep with the cycle.

4
  • How is this different then just ignoring it and coming back to it later? Nov 9, 2012 at 18:13
  • 1
    just small details: there's no extra "ignore me" test in reports, and if you use stashes, the code is indistinguishable from the 'clean' case.
    – Javier
    Nov 9, 2012 at 18:38
  • What is a stash? is that like version control? Nov 9, 2012 at 18:41
  • 1
    some VCS implement it as a feature (at least Git and Fossil). It allows you to remove a change but save it for reapplying some time later. It's not hard to do manually, just save a diff and revert to the last state. Later you reapply the diff and keep on.
    – Javier
    Nov 9, 2012 at 18:49
9

On the TDD cycle:

In the "make the test pass" phase, you are supposed to write the simplest implementation that will make the test pass. To make your test pass, you've decided to create a new collaborator to handle the missing logic because it was maybe too much work to put in your point class to make your test pass. That's where the problem lies. I suppose the test you were tying to make pass was a too much big step. So I think the problem lies into your test itself, you should delete/comment that test, and figure out a simpler test that will allow you to take a baby step without introducing the LineSegment.Intersects(LineSegment) part. One you have that test passing, you can then refactor your code (Here you will apply the SRP principle) by moving this new logic into a LineSegment.Intersects(LineSegment) method. Your tests will still pass because you won't have changed any behaviour but just moved some code around.

On your current design solution

But to me, you have a more profound design issue here is that you are violating the Single Responsibility Principle. The role of a Point is....to be a point, that's all. There is no smartness in being a point, it's just and x and y value. Points are value types. This is the same things for Segments, segments are value types composed of two points. They can contain a bit of "smartness" for example to compute their length based on their points position. But that's it.

Now deciding if a point and a segment are colliding, is a whole responsibility on its own. And is certainly too much work for a point or segment to handle on its own. It can't belong to the Point class, because otherwise Points will know about Segments. And it can't belong to Segments because Segments already have the responsibility to take care of the points within the segment and maybe also computing the length of the segment itself.

So this responsibility should be own by another class like for example a "PointSegmentCollisionDetector" which would have a method like:

bool AreInCollision(Point p, Segment s)

And that's something that you would test seperately from Points and Segments.

The nice thing with that design is now you could have different implementation of your collision detector. So it would be easy for example to benchmark your game engine (I assume you are writing a game:p) by switching your collision detection method at runtime. Or to do some visual checking/experiments at runtime between different collision detection strategies.

At the moment, by putting this logic in your point class, you are locking down things and pushing too much responsibility on the Point class.

Hope it makes sense,

1
2

The easiest thing to do in a TDD fashion would be to extract an interface for LineSegment and change your method parameter to take in the interface. Then you could mock the input line segment and code/test the Intersect method independently.

1
  • 2
    I know that that is the TDD method that I hear the most, but an ILineSegment doesn't make sense. It's one thing to interface out an external resource or something that could come in many forms, but I can't see one reason I would ever attach any of the functionality to anything other then a line segment. Nov 9, 2012 at 18:16
0

With jUnit4 you can use the @Ignore annotation for the tests you want to postpone.

Add the Annotation to each method you want to postpone, and continue writing tests for the required functionality. Circle back to refactor the older test cases later.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.