I LifetimeManager

Feb 26, 2008 at 11:52 PM
Edited Feb 26, 2008 at 11:53 PM
I know I know. Interfaces and versioning. However if that stands true, there's plenty of refactoring to be done in OB.

I'd vote for ILifetimeManager so it is easier to decorate or mock.

And I'd rename ClearValue to RemoveValue as it is a well known and vastly used term.
Feb 27, 2008 at 3:13 AM
I'd really like your opinion on the lifetime manager abstraction before discussing interface vs. abstract class. It looks like it's what we need to me, but other opinions are important on this one.
Feb 27, 2008 at 4:01 AM
Edited Feb 27, 2008 at 4:03 AM
Yeah you're right.

For what it is right now:

  • In the long run, I think it will be confusing for people to differentiate between SetSingleton(), RegisterInstance() and SetLifetime() but I see where it comes from.
  • In UnitDefaultBehaviorExtension, I'd reuse the same instance of SingletonLifetimeManager as there is no need to create a new one all the time (not that it really matters performance wise)
  • In SpecificInstanceLifetimeManager, I understand the flow but the "hack" to root the instance until Initialize is called and the release by setting the private member to null is a design smell to me
  • I'm wondering whether it would make sense or not to have a RegisterInstance that takes an optional LifetimeManager to control its lifetime so it isn't controlled by ILifetimeContainer.
  • I'd see TransientLifetimeManager as being registered as the default policy.
  • Having to provide the ExtensionContext to Initialize is an indicator that LifetimeManager is closer to an extension than you'd like to agree :)
  • The more I think about it, the more I think SpecificInstance doesn't make sense. It is the main (and probably only) reason why you had to add LifetimeManager.Initialize. It doesn't suit the model well.


If you get closer to what we talked before, you'd get something like:

  • Keep only SetLifetime(Type type, string name, LifetimeManager manager) + generic overloads, remove SetSingleton
  • Change slightly RegisterInstance to: RegisterInstance(Type type, object instance, string name, LifetimeManager manager) + generic overloads
  • Make Singleton and Transient lifetime managers singletons so we can have an .Instance syntax and don't have to new them all the time

The code for RegisterInstance would simply call the lifetimeManager.SetValue(...)

public void RegiserInstance(Type type, object instance, string name, LifetimeManager manager)
{
  SetLifetime(type, name, manager);
  manager.SetValue(instance); //TODO We don't have access to the IBuilderContext here. Does it really make sense to require IBuilderContext?
}

Of course, I see an overload of RegisterInstance without a LifetimeManager that uses SingletonLifetimeManager.Instance

At least that's what I see for now... Thoughts?
Feb 29, 2008 at 5:52 AM
I think I see where you're going with RegisterInstance,and I think I like it.

So, instead of SpecificInstanceLifetimeManager, we'd instead have "ContainerControlledLifetimeManager" (strong reference) and "ExternalLifetimeManager" (weak reference) or something like that? SpecificInstance would just go away.

The ExtensionContext and IBuilderContext were passed to the lifetime manager so that they'd have access to the locator, lifetime container, and policy list. With the above change, we don't need the locator anymore. We could have the container add the lifetime manager to the lifetime container on registration if it implements IDisposable. The policy list isn't needed by anything we've written so far; perhaps that's unneeded generalization? If that's the case, we can remove these parameters and the Initialize method completely.

SetSingleton is staying because our docs guy thought it made the container simpler. When the guy in charge of explaining things says this, you listen. ;-) Also, I'd rather not make SingletonLifetime a singleton; I like the "one manager instance per registration" model. Since it doesn't actually make any difference perf-wise.

So we remove the bool "containerControlsLifetime" from RegisterInstance, and instead it takes a LifetimeManager.

I like it. I'll play with it, and some of these changes should be in the next drop.

-Chris
Feb 29, 2008 at 6:08 PM
I wish I had a "Doc guy" :)

Some more feedback

  • I'm not sure I see the difference between SingletonLifetimeManager and ContainerControlledLifetimeManager. To me they both scope the lifetime of the referenced object to the lifetime of the container. That's why in the past I proposed "PerContainerLifetimeManager".
  • I think that you can safely remove the context from the lifetime manager methods. If some manager requires the context, it can receive it in its constructor. I already created an extension that "exposes" the ExtensionContext.
Feb 29, 2008 at 7:43 PM

francois_tanguay wrote:
I wish I had a "Doc guy" :)

Some more feedback

  • I'm not sure I see the difference between SingletonLifetimeManager and ContainerControlledLifetimeManager. To me they both scope the lifetime of the referenced object to the lifetime of the container. That's why in the past I proposed "PerContainerLifetimeManager".

There isn't any difference, and SingletonLifetimeManager is gone.


  • I think that you can safely remove the context from the lifetime manager methods. If some manager requires the context, it can receive it in its constructor. I already created an extension that "exposes" the ExtensionContext.



I did this last night, actually. The initialize method is about to go bye-bye as well.

-Chris
Mar 3, 2008 at 4:30 PM
I think you guys ought to take a community poll to guide the decision of whether to keep SetSingleton() or not. Unfortunately, the Unity codeplex site probably won't get a lot of traffic until Unity is already released. My vote is still "consistency over convenience" ... do all lifetime management through Extensions. I understand the facade argument, but Unity is only a facade because Object Builder is a framework. I wonder would the IUnityContainer be evolving differently if there were no Object Builder to begin with.

Derek
Mar 3, 2008 at 7:40 PM
New API coming tomorrow. SetSingleton is history.