tags:

views:

163

answers:

6

I'm learning test-driven development, and I have noticed that it forces loosely coupled objects, which is basically a good thing. However, this also sometimes forces me to provide accessors for properties I wouldn't need normally, and I think most people on SO agree that accessors are usually a sign of bad design. Is this inevitable when doing TDD?

Here is an example, the simplified drawing code of an entity without TDD:

class Entity {
    private int x;
    private int y;
    private int width;
    private int height;

    void draw(Graphics g) {
        g.drawRect(x, y, width, height);
    }
}

The entity knows how to draw itself, that's good. All in one place. However, I'm doing TDD, so I want to check whether my entity was moved correctly by the "fall()" method I am about to implement. Here is what the test case could look like:

@Test
public void entityFalls() {
    Entity e = new Entity();
    int previousY = e.getY();
    e.fall();
    assertTrue(previousY < e.getY());
}

I have to look at the object's internal (well, at least logically) state and see if the position was updated correctly. Since it's actually in the way (I don't want my test cases to depend on my graphics library), I moved the drawing code to a class "Renderer":

class Renderer {
    void drawEntity(Graphics g, Entity e) {
        g.drawRect(e.getX(), e.getY(), e.getWidth(), e.getHeight());
    }
}

Loosely coupled, good. I can even replace the renderer with one that displays the entity in a completely different way. However, I had to expose the internal state of the entity, namely the accessors for all of its properties, so that the renderer could read it.

I feel that this was specifically forced by TDD. What can I do about this? Is my design acceptable? Does Java need the "friend" keyword from C++?

Update:

Thanks for your valuable inputs so far! However, I fear I have chosen a bad example for illustrating my issue. This was completely made up, I will now demonstrate one that is closer to my actual code:

@Test
public void entityFalls() {
    game.tick();
    Entity initialEntity = mockRenderer.currentEntity;
    int numTicks = mockRenderer.gameArea.height
                   - mockRenderer.currentEntity.getHeight();
    for (int i = 0; i < numTicks; i++)
        game.tick();
    assertSame(initialEntity, mockRenderer.currentEntity);
    game.tick();
    assertNotSame(initialEntity, mockRenderer.currentEntity);
    assertEquals(initialEntity.getY() + initialEntity.getHeight(),
                 mockRenderer.gameArea.height);
}

This is a game loop based implementation of a game where an entity can fall down, among other things. If it hits the ground, a new entity is created.

The "mockRenderer" is a mock implementation of an interface "Renderer". This design was partly forced by TDD, but also because of the fact that I'm going to write the user interface in GWT, and there is no explicit drawing in the browser (yet), so I don't think it is possible for the Entity class to assume that responsibility. Furthermore, I would like to keep the possibility of porting the game to native Java/Swing in the future.

Update 2:

Thinking about this some more, maybe it's okay how it is. Maybe it's okay that the entity and the drawing is separated and that the entity tells other objects enough about itself to be drawn. I mean, how else could I achieve this separation? And I don't really see how to live without it. Even great object-oriented programmers use objects with getters/setters sometimes, especially for something like an entity object. Maybe getter/setter are not all evil. What do you think?

+4  A: 

The Pragmatic Programmers discuss tell, don't ask. You don't want to know about the entity, you want it to be drawn. Tell it to draw itself on the given Graphics.

You could refactor the code above so that the entity does the drawing, which is useful if the entity isn't a rectangle but actually a circle.

void Entity::draw(Graphics g) {
     g.drawRect(x,y, width, height);
} 

You'd then check that g had the correct methods called on it in your tests.

Paul Rubel
As explained above, I used to have the draw method by the entity, but I deliberately moved it away to avoid tight coupling with the graphics system.
Noarth
I think that's a problem with the Graphics abstraction, not the Entity abstraction. Like in the MVC you want a layer of abstraction between the how(concrete) and the what(abstract) of your graphics.
Paul Rubel
A: 

The thing you want to test is how your object will respond to certain calls, not how does it work internally.

So it's not really necessary (and it would be a bad idea) to access not accessible fields/methods.

If you want to see the interaction between a method call and one of the argument, you should mock the said argument in a way to be able to test if the method worked as you wanted to.

Colin Hebert
+2  A: 

So, had you not moved the draw(Graphics) method from Entity you had perfectly testable code. You need only inject an implementation of Graphics which reported the internal state of Entity to your test harness. Just my opinion though.

Tim Bender
A: 

First of all, are you aware of the java.awt.Rectangle class which is how this exact problem is dealt with in the Java Runtime Library?

Secondly, I believe that the real values of TDD is first that it moves your focus away from the "how do I do this specific detail with the data I assume is present like this" to "how do I call the code and what result do I expect back". The traditional approach is "fix the details, and then we'll figure out how to call the code", and doing it the other way around allows you to find faster if anything simply cannot be done or not.

This is very important in designing API's, which is most likely also why you have found that your result is loosely coupled.

The second value is that your tests are "living comments" as opposed to ancient, untouched comments. The test shows how your code should be called, and you can instantly verify that it behaves as specified. This is not relevant to what you ask, but should demonstrate that tests have more purposes then simply to blindly call some code you wrote a while back.

Thorbjørn Ravn Andersen
+1  A: 

You say that you feel the Renderer class you came up with was "specifically forced" by TDD. So, let's look at where TDD led you. From a Rectangle class that was responsible for its coordinates and for drawing itself, to a Rectangle class that has the Single Responsibility of maintaining its coordinates and a Renderer that has the Single Responsibility of, well, rendering a Rectangle. That's what we mean when we say Test Driven - this practice affects your design. In this case, it drove you to a design that more closely adhered to the Single Responsibility Principle - a design you wouldn't have gone to, without the tests. I think that's a good thing. I think you're practicing TDD well, and I think it's working for you.

Carl Manaster
A: 

I think your example has a lot in common with the example used here:

http://www.m3p.co.uk/blog/2009/03/08/mock-roles-not-objects-live-and-in-person/

Using your original example and replacing the Entity with Hero, fall() with jumpFrom(Balcony), and draw() as moveTo(Room) it becomes remarkably similar. If you use the mock object approach Steve Freeman suggests, your first implementation was not so bad after all. I believe @Colin Hebert has given the best answer when he pointed in this direction. There's no need to expose anything here. You use the mock objects to verify whether the behavior of the hero has taken place.

Note that the author of the article has co-written a great book that may help you:

http://www.amazon.com/Growing-Object-Oriented-Software-Guided-Tests/dp/0321503627

There are some good papers freely available as PDF by the authors about using mock objects to guide your design in TDD.

koen
I don't think that I can use a mock object for drawing, because I will implement a renderer for GWT. I cannot really create a Graphics-like object for GWT, that wouldn't work because there is no drawing in GWT, there is just modification of DOM elements, and I don't want DOM elements in my core game logic package.
Noarth
Furthermore, I don't think that the draw method is the same like the moveTo method. My draw method is low-level display drawing, while moveTo is a logical operation, one I would test.
Noarth