4

I'm working on a large codebase that has made extensive use of the Singleton pattern, as well as some Globals. I've just started to try and write some unit tests, but Singletons and Globals are causing me lots of problems, and after reading up, Dependency Injection would seem the way to go.

The refactoring task ahead to make this change is very daunting, and I'm trying to work out the best approach. As far as I understand the basic idea is to take something like this:

foo()
{
    GraphicsCache::Instance()->GetMyImage();
    // do stuff
}

and turn it into something like this:

foo(GraphicsCache *Cache)
{
    Cache->GetMyImage();
    // do stuff
}

That way I can make mocks of these objects and use the mocks in my tests. But there are lots of these types of objects (event loggers, network objects, other caches etc.), which pretty much means I'm going to end up passing a lot of objects around all over the place, and will end up adding a lot of code all over the place. Have I got the right idea? Is there a better way to do this?

One idea I had was to have a single object that is a container of all of these currently global objects, and all I have to do is pass around a single reference, but that doesn't feel right as most places won't need access to every single global object, just a sub set.

oggmonster
  • 4,672
  • 10
  • 51
  • 73
  • Re: your idea of passing round a single object. You are just passing round a pointer to the global object. It doesn't matter whether the other routines need them or not: you are not passing around huge amounts of data: just one pointer. The information is there but they don't have to use it. It is like carrying round a mobile phone with features that you never use. They are there but you don't have to use them. The phone won't be any lighter without those features. – cup Jan 10 '14 at 17:22

2 Answers2

3

You should do what you suggested, passing the objects around is the right way to go. You may want to use factories too. And yea, you will end up with adding alot of code, but it's worth it.

Don't create a "package" object to be passed around, from what iv'e seen it creates the exect same problems as global variables, you can't tell what your objects really depend on, as everything is being passed to them.

If you really don't have that much time, or it might not work for some other reason (like you'er manager is unhappy with this), you can also create dedicated factories and change the object that the factory creates from the testing.

You can read more about it, it's called Interface Segregation Principle

Simply putting, the principle states that a class should not depend on functions it does not use.

user1708860
  • 1,683
  • 13
  • 32
  • That makes a lot of sense, I understand why a "package" would be bad. The code base is just a bit of a mess because singletons mean you can get away with refusing to confront the software architecture issues, and it's very hard to sort out after the fact! – oggmonster Jan 10 '14 at 17:32
  • Also, pretty much every single class needs the event logger, so surely this would mean adding an event logger constructor argument to every class in the code base. That does seem a bit crazy? – oggmonster Jan 10 '14 at 17:34
  • @MatthewHubble we acctually ran into the event logger problem aswell in my work place, What we ended up doing is either creating the event logger in the constructors (using a factory ofcourse, bus still a bad idea) and still some of the classes depended on a global event logger. This might be helpfull - http://stackoverflow.com/questions/20830673/different-loggers-used-with-libraries. We could not find an easy solution for the logging problem... – user1708860 Jan 10 '14 at 17:37
1

I'm answering this from a c# perspective, but it should be equally applicable for C++. Objects are definitely better than singletons, but for true cross cutting concerns (i.e. Logging etc...) you may want to consider modifying those singletons into the AmbientContext pattern. Its basically a singleton on steroids, but you instantiate it on application/test startup to an interface implementation of you're choice. This would make the loggers fully testable, whilst not causing you to have to inject them into everything.

Ambient contexts may also serve as a temporary middle ground to allow testing to prevent having to refactor out all those singletons in on go.

gmn
  • 4,199
  • 4
  • 24
  • 46