(I originally posted this on my MSDN blog.)
IoC containers are awesome and I use them in all my non-trivial projects. However, there’s an interesting caveat when it comes to using IoC containers that isn’t totally obvious: don’t get domain entities straight from the container.
Surprisingly, this rule doesn’t seem to have a lot of source material. It’s well-known among the IoC experts (you might run into terse “don’t do that” comments on mailing lists and discussion boards) but I’ve had a really hard time finding a clear, concise explanation for exactly why getting domain objects from the container is an anti-pattern. Maybe my web search kung fu is weak, I dunno. This post is about the best I’ve been able to find and even it doesn’t do into the reasons behind the rule. If anyone has a better source, let me know.
Unfortunately I’m not going to write that definitive explanation either but I can at least demonstrate what happened when I ignored it. I already knew that domain objects don’t belong in the container before I started writing GenesisEngine but for some reason I wasn’t really thinking of the Planet class (and all of its component classes) as domain objects. I’m not sure why; probably because I started with the assumption that there would be one and only one planet, but of course it’s really a domain entity because it has uniquely identifying information (its location and radius).
So what happened?
How it was
My MainPresenter was created from the container and received an injected IPlanet from the container, like so:
public MainPresenter(IPlanet planet, ICamera camera, IWindowManager windowManager,
Statistics statistics, ISettings settings)
{
_planet = planet;
_planet.Initialize(DoubleVector3.Zero, PhysicalConstants.RadiusOfEarth);
_camera = camera;
_windowManager = windowManager;
_statistics = statistics;
_windowManager.ShowAllWindows();
_settings = settings;
_settings.ShouldUpdate = true;
}
The container created the Planet object but it couldn’t give it any uniquely identifying information because the container didn’t know anything about what kind of planet I wanted to create. So in the MainPresenter constructor, I had to call IPlanet::Initialize() to pass in the unique attributes. This two-stage construction is a definite anti-pattern because there’s no safety net to prevent the consumer of IPlanet from forgetting to initialize the injected dependency.
Here’s the construction and initialization of the planet class where you can see the two-stage construction. Note that the concept of “inject then initialize” was cascading to the Planet dependencies like the PlanetRenderer as well.
public Planet(IPlanetRenderer renderer, ITerrainFactory terrainFactory,
IHeightfieldGenerator generator, Statistics statistics)
{
_renderer = renderer;
_terrainFactory = terrainFactory;
_generator = generator;
_statistics = statistics;
_clippingPlanes = new ClippingPlanes();
}
public void Initialize(DoubleVector3 location, double radius)
{
_location = location;
_radius = radius;
CreateTerrain();
_renderer.Initialize(_radius);
}
Aside from obtuse code that was likely to break, my main problem was that I could only have one planet in my program. What if I wanted to put a moon in orbit around my planet, or model a solar system? Clearly injecting planets from the container into the MainPresenter wasn’t the right approach. I guess I could have had the MainPresenter grab additional planets straight from the container itself, but it’s a code smell to reference the container outside of bootstrapper/setup code.
Interestingly, this poor design was indirectly causing bizarre problems in other seemingly-unrelated areas of the code. For instance, I had this comment in my initialize method for the main Genesis program class:
protected override void Initialize()
{
this.IsMouseVisible = true;
_mainPresenter = ObjectFactory.GetInstance<MainPresenter>();
// TODO: we really don’t need a reference to the camera controller here but
// no one else seems to need a reference to it either and it needs to be
// created otherwise the event aggregator won’t subscribe it. Is there a
// better way to handle this?
_cameraController = ObjectFactory.GetInstance<ICameraController>();
_inputState = ObjectFactory.GetInstance<IInputState>();
_inputMapper = ObjectFactory.GetInstance<IInputMapper>();
SetInputBindings();
base.Initialize();
}
I had a CameraController object that needed to exist but nothing in the whole app seemed to have a reference to it. I had to hack in a reference in my main program class just to force the object to be created and to stay around. Turns out that problem was caused by my pulling domain objects out of the container. The IPlanet was being directly injected into the CameraController when it should have gone a different route.
How I fixed it
Fixing this kind of mess usually involves factories.
Rather than injecting the IPlanet from the container I injected an IPlanetFactory into the MainPresenter. MainPresenter can now use the planet factory to create as many planets as it needs. It turns out that MainPresenter now has kind of a two-step creation process – construct it then show it, but that makes logical sense for a presenter and actually helps straighten out the flow of showing other windows, etc.
public MainPresenter(IPlanetFactory planetFactory, ICamera camera, ICameraController cameraController, IWindowManager windowManager, Statistics statistics, ISettings settings)
{
_planetFactory = planetFactory;
_camera = camera;
_cameraController = cameraController;
_windowManager = windowManager;
_statistics = statistics;
_settings = settings;
_settings.ShouldUpdate = true;
}
public void Show()
{
_planet = _planetFactory.Create(DoubleVector3.Zero, PhysicalConstants.RadiusOfEarth);
_cameraController.AttachToPlanet(_planet);
_windowManager.ShowAllWindows();
}
Note that the CameraController is injected into the MainPresenter and is explicitly attached to a particular planet which fixed my phantom CameraController problem since now the container actually needs to create it and MainPresenter holds a reference to it. The bizzaro reference I had in the Genesis program class went away and the handling of MainPresenter is more logical (get it from the container and then show it).
protected override void Initialize()
{
this.IsMouseVisible = true;
_mainPresenter = ObjectFactory.GetInstance<MainPresenter>();
_mainPresenter.Show();
_inputState = ObjectFactory.GetInstance<IInputState>();
_inputMapper = ObjectFactory.GetInstance<IInputMapper>();
SetInputBindings();
base.Initialize();
}
Finally, the Planet class is simpler and there’s no cascading Initialize calls to the renderer any more:
public Planet(DoubleVector3 location, double radius, ITerrain terrain,
IPlanetRenderer renderer, IHeightfieldGenerator generator, Statistics statistics)
{
_location = location;
_radius = radius;
_terrain = terrain;
_renderer = renderer;
_generator = generator;
_statistics = statistics;
_clippingPlanes = new ClippingPlanes();
}
You can see the GitHub Compare View of all of the changes here.
SaintGimp 
One thought on “GenesisEngine: Don’t Get Domain Objects From The Container”