mardi 25 avril 2017

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.

Aucun commentaire:

Enregistrer un commentaire