Skip to main content
8 of 18
added 469 characters in body
candied_orange
  • 119.7k
  • 27
  • 233
  • 369

This is not a God object.

It seems like it is because there is so much here, but, in a way, it's doing nothing at all. There is no behavior code here. This object isn't an omnipotent God that does everything. It just finds everything.

This pattern has a more proper name: Service Locator. It strongly contrasts with the Dependency Injection* pattern.

Your testablility complaint is valid but the main principle being violated here is the Interface Segregation principle.

Here's why: If I come at this thing with an intent to refactor away the Dependency Inversion problem I'd just stop hard coding CoreService. I'd replace it with a parameter that must be passed in. Can you see why that doesn't really help?

On top of it being annoying seeing this parameter passed into everything, this doesn't actually fix the dependency problem because seeing that an object needs CoreService tells me zero about it's real needs because CoreService provides access to anything and everything. We externalize dependencies to make needs clear.

If we follow the Interface Segregation principle we see that an object that, right now, needs CoreService actually needs maybe 5 out of the 100 things it provides. What those 5 things need is a good descriptive name.

Now when I see that name I know what this thing needs. I can go find ways to provide those 5 things. There might be 2 or 42 ways to provide those 5 things. One of those ways might be through a test, but this idea is about a lot more than testing. Testing just makes this problem painfully obvious.

To provide that name, and avoid a constructor with 5 parameters, you can introduce a parameter object*, ** provided those 5 things represent one coherent idea with, please, a good name. (It may be 2 ideas that need 2 names are hiding in those 5 things. If so fine, break them up and name em.

You might be tempted to reuse this parameter object if you find another object that needs it. But let's say that object needs something else as well. So you add that something else to the parameter object. Even though the first object doesn't need this something else. Well we're now on our way back to making a Service Locator. You stop this by not accepting things you don't need. That's what the Interface Segregation principle is all about.

Another pattern hiding in here seems to be Singleton*. Dependency Injection has a wonderful alternative to that. Just build what you need in main once and pass references to what you built to everything that needs it. Once you have built your graph of long lived objects call one method on one of them to start the whole thing ticking. Just try not to go nuts. It's OK to break this up with factories and other creational patterns. Keep creation and behavior code separate.

To convince others you're going to have to start showing how this problem can be fixed. Make some things that express their needs and write code that satisfies those needs. A simple factory with a good name and it's own file to live in often gets this done. If you're stuck with the need to find the rest of their stuff let the factory deal with that problem. Now your object is blissfully unaware of their nonsense and won't care when you get around to fixing the rest of it.

It also news up its dependencies, which is bad.

That's a little simplistic. It's bad to "new up dependencies" when you don't provide a way to override them. You seem to be using C#. C# has named arguments! Which means you can satisfy a dependency with an optional argument. Just be sure you have a good default value for it. Good default values should be chosen wisely. Don't use ones that surprise people.

candied_orange
  • 119.7k
  • 27
  • 233
  • 369