I am sitting at the airport at the moment, having to burn some time before I can get on a flight, and I decided to make use of this time to review the Unity container from the P&P group.
A few things before I start. Unity is a CTP at the moment, it is not done, and that is something that should be taken into account. I have several design opinions that conflict with the decisions that were made for Unity. I am a contributor to Windsor. Overall, I am biased. Please take that into account.
I am also going to put it through some of the paces that I put Windsor through. Just to see how it goes. Again, I am comparing a > 4 years old container to a very new new, another thing to keep in mind. I do not foresee a situation in which I will use Unity over Windsor, but I do hope that this post will be helpful in future improvements to Unity.
A lot of this feedback is based on my experience and may contain comments that can be considered personal preferences.
Let us start with the things that I like:
- This piece of code is very nice, and it is useful in many scenarios where the instance is created by an external source (Page instance, Forms, Web Services, and many more). At the moment, this is something that Windsor doesn't have (I need two find a few hours free on a machine that has a development environment.
myContainer.BuildUp<MyRealObject>(myObject); - I am ambivalent as far as the Method Injection goes, in the abstract, I say cool, nice feature. The practical side says that it is mostly not required. Nevertheless, let us put it here.
- The XML configuration is very clean and easy to follow. At least the sample ones that I have seen. I am not sure if it is possible to put it in an external file (not in app.config), but that is probably my only reservation with that.
- The ability to configure instances from the configuration is cute. Especially if you add the converter support that you have there. It doesn't seem, at least from a brief glance, that you can build complex objects there, though. That is not a bad thing, however :-)
- It was a really good decision, to decouple the container and its configuration.
And now to the things that I don't like:
- The documentation (and a feature which attempts to resolve a type by default, even if it is not registered) direct toward having concrete class dependencies. I strongly dislike this, I think that except for rare cases, an interface should be used. And I generally dislike this feature.
- For that matter, the ability to resolve a type that wasn't registered in the container strikes me as problematic. There were some long discussions on that in the ALT.Net mailing list, and I can see the other side's point of view. Yes, the container should enable this feature, but not in the common scenario. Preferably, add a method that make it explicit what you are doing.
- According to the documentation, having several constructors will result if Unity picking one of them at random, and supplying its dependencies. I find the greediest possible ctor approach much more predictable, and a more natural model.
- [DependencyConstructor] bothers me. This is the container invading into the application in an unseemly manner. It bothers me more that it is not optional if you have more than a single ctor.
- Much worse, from the point of view of container ignorance, is the facilities for property injection. Here you have to specify [Dependency] on all the properties that you want injected. Again, this is the container reaching into my code and messing with that in unseemly ways. My code should have nothing to do with the container whatsoever. By the shape of my object, so will the container work. Not the other way around.
- container.Get<ISomethingMadeUp>() will return a null reference if it can't create the type. I think that this is the wrong design decision. It should definitely throw here. It means that instead of getting a nice exception "ISomethingMadeUp is not registered" I get "Object not set to an instance of an object". While the ability to try resolve from the container is useful, it should be in a separate method. TryGet would be my preference for that.
- Introspection capabilities are missing. That is, there is no way (that I could easily see, and I looked for one, to ask if a given type or a given extension are registered in the container. This is critical for many advance scenarios.
- Something that I consider to be a sever bug, but from the code looks like a design decision, is ignoring missing dependencies when you resolve a component. That is, assume that you component Foo that have constructor dependency on ILogger. If you resolve Foo, and the container will not be able to find an ILogger implementation, it will pass null to the constructor. Read that one again. Instead of getting "Cannot resolve Foo because ILogger cannot be constructed" you are going to get "Object reference not set to an instance of an object".
This is a problem, period. - Yes, you can use [Dependency(NotPresentBehavior)] to change that, but that is again, an invasive approach. Not a solution.
- There is no protection from cycles in the dependency graph A depends of B depends on C depends on A. Again, you need to get a good exception here, explaining the problem. What you get is a stack overflow exception and bye bye to the program.
- No support for generic components. That is, registering IFoo<> and getting IFoo<string>.
- At a certain point, calling a method on the container will raise an event that will be handled by a strategy. Is seems like a very odd thing to me. The case in point is setting a component to be a singleton, I would have grabbed the singleton strategy directly and called that, instead of going through the event.
- Life time support. Right now Unity supports two levels, singleton and transient. That is good, and you can probably extend that with policies. The problem is that those ideas are hard coded into the container interface. Off hand, you need at least local (per thread or per request) models as well. And I think that exposing singleton at the container interface is a mistake.
- I haven't seen how you can control the dependencies yet, which is a very important concern. That is, when I am building EmailSender, I want it to use port 435 and the file logger, instead of the email logger that everything else uses. This is a key feature, but reading the forums, this is currently a problematic feature.
Object Builder 2 Observations:
- I like the use of dynamic methods to create instances, but the implementation (at least at first glance) seems awfully complicated. The main object that I have here is that you have the control flow of generating this method separated over a large number of objects and interfaces. The advantage that this give you is fine grain control over this process, but consider the task involved, this is a highly cohesive process, and you can't really just plug your own stuff to it without properly understanding everything that goes there.
- In the same vein, the use of policies and strategies for everything obscure the intent of the code. The DynamicMethodConstructorStrategy, for example, need to get the generation context, the existing parameter is use for that, which seems like a bad abuse of the given API. I would create an independent component with an API that would explicitly make this dependency. If you wanted the ability to replace that, have it registered in the container by default, then replace that before you resolve anything.
- In the above case specifically, we have the existing parameter that sometimes holds the existing object, and sometimes holds the current context. That is not a friendly API.
I wanted to try Unity's extensibility model, but I think that I put enough time in it already. What I have learned so far makes it pretty much pointless.
I wanted to build the StartableExtension, which automatically starts up components that implements the IStartable interface. On the surface, it is very simple, register to TypeRegister event, get the type and Start() it. The problem is that you need to wait until all the type's dependencies are available. For example, I may register IHealthChecker before I register ILogger, which it need. There is no way in Unity at the moment to enable this scenario.
Something else that I would like to see is Unity's version for Environment Validation. Here is is likely to be possible to do this, but it expose a problem with Unity's dependency model. You can't register arguments for a component, you can only do that globally, and that is if you are willing to perform invasive operation on your components.
Overall, I think that is an okay solution. You can see some Windsor influence in the API and in the configuration, which make me happy.
It has too many red lines there for me to be able to use it (even if Windsor wasn't around), however. Some of the things that I have outlined here (error handling, for example) can be fixed easily enough, I am much more concerned with the design decisions that were made. Specifically, not throwing when a dependency is not found, both in the specific case of directly calling Get() and in the general case of resolving a dependency for a type that is currently being resolved.
The invasiveness of the container is one of the top issues that I would have fixed. That is one of the major reasons that CAB got lambasted.
I remember having to work with Object Builder circa 2005 - 2006, and it was a major pain. Unity looks significantly better than that. As well as the bits of Object Builder 2 that I have examined. I still object to the strategies / policies design, however, it seems like an overly generic solution. And methods like AddNew offend me for some reason. Add( new Foo() ) isn't hard.
And, to save myself a lot of trouble later on, this is not an attack on Unity, this is simply me going over what is there and expressing my professional opinion on an early version of a project.