Singletons - architectural refactor

[wheybags]

So I want to start getting some tests into freeablo, specifically regarding serialisation for multiplayer and savegames, and CEL parsing.

One of the problems with doing this is going to be the multiple singletons we have, which make testing pretty difficult. We have a few options:

• Using some sort of service locator pattern (basically mash all the current singletons into one singleton, with methods like getRenderer(), getNetManager(), etc)
• Pass dependencies down through constructors.

I'm not really sure which of these is the best option, so I'm going to open it up for discussion here.

[konopka90]

In general creating tests in late phase of production should not have big impact on production code. Single test (or more often group of tests) should be treated as separated entity and maintained separately. It means that for every group of tests it is recommended to create proper stubs and mocks. Mock should provide minimum fake implementation that is required to launch and test one entity. In 99% one entity means one class from production code.

I didn't study freeablo code at all so it is imagined example: You want to test serialisation of Player but constructor of this class needs Renderer. In this situation you need Renderer mock with minimum implementation. Create separated folder for test , place here test suite with code (tests/tests.cpp) and mock for Renderer (tests/Renderer.cpp). During compilation of test suite use original headers and src from production code for Player class (production/Player.h, production/Player.cpp) and newly created source code of mock (tests/Renderer.cpp) with original header (production/Renderer.h).

I recommend study gmock and gtest documentation - very useful and handy. It can give fresh view on subject. Disadvantage or advantage of this approach is that tests and mocks/stubs should be maintainted as well as source code. Why advantage? Because in comparison with other approaches it can be more efficient and can save time.

[wheybags]

With gmock can we for example, test serialisation of Player using a mocked renderer, but then in a separate test, test the actual renderer? I've never actually used any unit testing tools in c/c++, just c# and js, which are obviously pretty different.

[konopka90]

With gmock can we for example, test serialisation of Player using a mocked renderer, but then in a separate test, test the actual renderer?

Yes, exactly. This is basics of TDD - separate single class as much as you can from external code and write required mocks. Scheme is identical for each class. What's more depending on testcase, behaviour of your mock can be different - you can decide what single function should return if x = 0 or if x=1. Going further if you want to change production code the best solution is to create interfaces for every production class (if it is only possible). Then, when you create mock you just use this interface and you know exactly what should be implemented. Next useful thing is Abstract factory. When you have interfaces for classes this approach gives you nice design pattern. You can have two factories - one for production code and one for testing. There are some examples in the Internet f.e: Look at final class diagram for better understanding: https://prashantbrall.wordpress.com/2010/11/12/interface-tdd-part-2-the-factory-pattern/

[konopka90]

If you decide to rewrite part of code it is connected with many changes but I think it is worth. What's more you can learn something new ;)

Edit: Very good example is here https://github.com/wheybags/freeablo/pull/155 . I provided interface Level so I was able to create implementation LevelImpl. For tests I'm able to create LevelMock which behaviour can be easily defined, for example bool isPassable can always return true.

[konopka90]

Regarding your original question on many sites people recommend to pass down dependencies and follow dependency injection design pattern.

[Fuco1]

Definitely go with DI. Service locator and singleton is basically the same thing, you couple the code to some external dependency you can't change later on (though with service locator you can theoreitcally provide a SL mock which will return mocks for everything else... we've done this once and it is unmaintainable hell :))