CODEWITHSUNDEEP
testingtdd
Ramon Amorim
Ramon Amorim

Reputation: 131

Best Practices for this TDD attempt

I code for about 12 years, but I've never get used to TDD in all this time.

Well, things are about to change, but since I'm learning all by myself, I hope you guys could help me.

I'm posting a game example for a VERY SIMPLE chest class. When the player grabs a chest, it registers the current time it was obtained. This chest takes some time to open so, I need, for UI reasons, to show the remaining time it takes to open. Each chest has a type, and this type is bound to a database value of how much time it would take to open.

This is a "no-testing-just-get-things-done-fast-mindset". Consider that ChestsDatabase and DateManager are singletons containing the database-bound values and the current system time wrapped into a class.

public class Chest {
    private readonly int _type;
    private readonly float _timeObtained;

    public Chest(int type, float timeObtained) {
        _type = type;
        _timeObtained = timeObtained;
    }

    public bool IsOpened() {
        return GetRemainingTime() <= 0;
    }

    // It depends heavily on this concrete Singleton class
    public float GetRemainingTime() {
        return ChestsDatabase.Instance.GetTimeToOpen(_type) - GetPassedTime();
    }

    // It depends heavily on this concrete Singleton class
    private float GetPassedTime() {
        return DateManager.Instance.GetCurrentTime() - _timeObtained;
    }
}

Of course, I could have made it in a Dependency Injection fashion and get rid of the singletons:

public class Chest {
    private readonly ChestsDatabase _chestsDatabase;
    private readonly DateManager _dateManager;
    private readonly int _type;
    private readonly float _timeObtained;

    public Chest(ChestsDatabase chestsDatabase, DateManager dateManager, int type, float timeObtained) {
        _chestsDatabase = chestsDatabase;
        _dateManager = dateManager;
        _type = type;
        _timeObtained = timeObtained;
    }

    public bool IsOpened() {
        return GetRemainingTime() <= 0;
    }

    public float GetRemainingTime() {
        return _chestsDatabase.GetTimeToOpen(_type) - GetPassedTime();
    }

    private float GetPassedTime() {
        return _dateManager.GetCurrentTime() - _timeObtained;
    }
}

What if I use interfaces to express the same logic? This is going to be much more "TDD-friendly", right? (supposing that I've done the tests first, of course)

public class Chest {
    private readonly IChestsDatabase _chestsDatabase;
    private readonly IDateManager _dateManager;
    private readonly int _type;
    private readonly float _timeObtained;

    public Chest(IChestsDatabase chestsDatabase, IDateManager dateManager, int type, float timeObtained) {
        _chestsDatabase = chestsDatabase;
        _dateManager = dateManager;
        _type = type;
        _timeObtained = timeObtained;
    }

    public bool IsOpened() {
        return GetRemainingTime() <= 0;
    }

    public float GetRemainingTime() {
        return _chestsDatabase.GetTimeToOpen(_type) - GetPassedTime();
    }

    private float GetPassedTime() {
        return _dateManager.GetCurrentTime() - _timeObtained;
    }
}

But how the hell am I supposed to test something like that? Would it be like this?

    [Test]
    public void SomeTimeHavePassedAndReturnsRightValue()
    {
        var mockDatabase = new MockChestDatabase();
        mockDatabase.ForType(0, 5); // if Type is 0, then takes 5 seconds to open
        var mockManager = new MockDateManager();
        var chest = new Chest(mockDatabase, mockManager, 0, 6); // Got a type 0 chest at second 6
        mockManager.SetCurrentTime(8); // Now it is second 8
        Assert.AreEqual(3, chest.GetRemainingTime()); // Got the chest at second 6, now it is second 8, so it passed 2 seconds. We need 5 seconds to open this chest, so the remainingTime is 3
    }

Is this logically right? Am I missing something? Because this seems so big, so convoluted, so... wrong. I had to create 2 extra classes ~just~ for the purpose of these tests.

Let's see with a mocking framework:

    [Test]
    public void SomeTimeHavePassedAndReturnsRightValue()
    {
        var mockDatabase = Substitute.For<IChestsDatabase>();
        mockDatabase.GetTimeToOpen(0).Returns(5);
        var mockManager = Substitute.For<IDateManager>();
        var chest = new Chest(mockDatabase, mockManager, 0, 6);
        mockManager.GetCurrentTime().Returns(8);
        Assert.AreEqual(3, chest.GetRemainingTime());
    }

I got rid of two classes with the framework, but still, I feel there's something wrong. Is there a simpler way in my logic? In this single case, would you use a mocking framework or implemented classes?

Would you guys get rid of the tests altogether or would you insist in any of my solutions? Or how to make this solution better?

Hope you can help me in my TDD journey. Thanks.

Upvotes: 2

Views: 89

Answers (2)

Krzysztof Jelski
Krzysztof Jelski

Reputation: 342

For your current design your last attempt is logically right and close to what I would consider an optimal test case.

I recommend extracting mock variables to field. I would also reorder test lines to have a clear distinction between setup, execution and verification. Extracting chest type to constant also makes the test easier to understand.

private IChestsDatabase  mockDatabase = Substitute.For<IChestsDatabase>();
private IDateManager mockManager = Substitute.For<IDateManager>();
private const int DefaultChestType = 0;

[Test]
public void RemainingTimeIsTimeToOpenMinusTimeAlreadyPassed()
{
    mockDatabase.GetTimeToOpen(DefaultChestType).Returns(5);
    mockManager.GetCurrentTime().Returns(6+2);
    var chest = new Chest(mockDatabase, mockManager, DefaultChestType, 6);

    var remainingTime = chest.GetRemainingTime();

    Assert.AreEqual(5-2, remainingTime);
}

Now for a more general comment. The main benefit of TDD is it gives you feedback on your design. Your feelings that the test code is big, convoluted and wrong are an important feedback. Think of it as a design pressure. Tests will improve both with test refactoring, as well as when the design improves.

For your code, I would consider these design questions:

  1. Are responsibilities assigned properly? In particular, is it Chest's reponsibility to know the passed and remaining times?
  2. Is there any concept missing in the design? Maybe each chest has a Lock, and there is a time-base Lock.
  3. What if we passed the TimeToOpen instead of Type to Chest upon construction? Think of it as passing a needle instead of passing a haystack, in which the needle is yet to be found. For reference, see this post

For a good discussion of how tests can provide design feedback, refer to the Growing Object Oriented Software Guided by Tests by Steve Freeman and Nat Pryce.

For a good set of practices for writing readable tests in C# I recommend The Art of Unit Testing by Roy Osherove.

Upvotes: 3

Umair Jameel
Umair Jameel

Reputation: 1673

There are some major points that are needed to be considered while writing unit tests as shown

  1. Separate project for unit testing.

  2. One class for writing unit tests of functions in one class of main code.

  3. Covering conditions within functions.
  4. Test Driven development (TDD)

If you really want to know more (with examples), have a look at this tutorial

Unit Tests c# - best practices https://www.youtube.com/watch?v=grf4L3AKSrs

Upvotes: 2

Related Questions