5
\$\begingroup\$

One of the tenets of Windsor IoC (probably applies to all IoC containers too) is to "release what you explicitly resolve", which admittedly should occur rarely. But we have a fair few UsingFactoryMethod setups in our installers, which are resolving directly (which appears to be a valid case for explicit resolution).

However, remembering to call Dispose() on those resolved things is something I doubt a lot of people will remember, so I came up with a helper method in order to be able to wrap that logic away (always a good thing, right?). However, I'm not sure if it's just a bit overkill in this case.

namespace Castle.Windsor {
    using System;
    using Castle.MicroKernel;

    public static class KernelExtensions {

        public static ResolvedResult<TInstance> ResolveDispose<TInstance>(this IKernel kernel) {
            return new ResolvedKernelResult<TInstance>(kernel);
        }

        public static ResolvedResult<TInstance> ResolveDispose<TInstance>(this IWindsorContainer container) {
            return new ResolvedContainerResult<TInstance>(container);
        }

        public abstract class ResolvedResult<T> : IDisposable {

            public T Instance { get; private set; }

            protected ResolvedResult(Func<T> resolver, Action<object> releaser) {
                this.Instance = resolver();
                this._Releaser = releaser;
            }

            #region IDisposable Members

            public void Dispose() {
                if (null == _Releaser) {
                    throw new ObjectDisposedException(this.GetType().Name);
                }

                //Dis-associate the resolved instance, can't null assign as T isn't typed
                Instance = default(T);

                _Releaser(Instance);
                _Releaser = null;
            }

            #endregion

            private Action<object> _Releaser;

            public static implicit operator T(ResolvedResult<T> res) {
                return res.Instance;
            }
        }

        private sealed class ResolvedKernelResult<T> : ResolvedResult<T> {
            internal ResolvedKernelResult(IKernel kernel) : base(kernel.Resolve<T>, kernel.ReleaseComponent) { }
        }

        private sealed class ResolvedContainerResult<T> : ResolvedResult<T> {
            internal ResolvedContainerResult(IWindsorContainer container) : base(container.Resolve<T>, container.Release) { }
        }
    }
}

Usage

Before:

container.Register(
    //...

    Component.For<MyComponentType>()
        .UsingFactoryMethod(k => {
            var altComponent = k.Resolve<SomeAlternativeComponent>();

            var component = new ImplementingComponentType(altComponent.RandomProperty);

            k.ReleaseComponent(altComponent);

            return component;
        })
)

After:

container.Register(
    //...

    Component.For<MyComponentType>()
        .UsingFactoryMethod(k => {
            using(var altComponent = k.ResolveDispose<SomeAlternativeComponent>()) {
                return new ImplementingComponentType(altComponent.Instance.RandomProperty);
            }
        })
)
\$\endgroup\$
2
  • \$\begingroup\$ Is it easy to give a short example of releasing objects correctly with and without using this extension? \$\endgroup\$ Jul 18, 2014 at 15:13
  • \$\begingroup\$ @BenAaronson Sure, see updated question \$\endgroup\$
    – Psytronic
    Jul 18, 2014 at 16:55

2 Answers 2

3
\$\begingroup\$
  1. Apparently passing in null for the release action is not valid so in your constructor you should check and throw an ArgumentNullException if that is the case.

  2. I might be missing something, but shouldn't this be called the other way round:

    Instance = default(T);
    
    _Releaser(Instance);
    

    Right now you release null or a default empty object in case of value types rather than the actual instance.

\$\endgroup\$
1
  • \$\begingroup\$ Thanks, that was an on-the-fly improvement I did, you're correct should be the other way around, updated \$\endgroup\$
    – Psytronic
    Jul 19, 2014 at 18:57
3
\$\begingroup\$

Style-wise everything looks good. The only note I'd make is that in C#, generally the convention is to start brackets on a new line rather than in-line, although obviously this is ultimately up to personal or team preference. XML comments on public members may be a good idea too, as it's not trivially obvious what the purpose of this class is unless you're very familiar with the container.

As to the actual usefulness of the helper, though, I'm doubtful:

  • It doesn't significantly reduce the amount of code you have to write
  • It is just as reliant on people remembering to use it
  • It does not improve readability, and if anything, slightly degrades it.

One alternative would be instead of providing a helper method to use inside UsingFactoryMethod, you provide an alternative version of that method. You'd probably need to tweak it, but a rough version might look like:

public static ComponentRegistration<TService> UsingFactoryMethod<TService, TResource, TImpl>(
        this ComponentRegistration<TService> component, Func<IKernel, TResource> resourceResolver,
        Func<IKernel, TResource, TImpl> factoryMethod)
        where TService : class
        where TImpl : TService
    {
        return component.UsingFactoryMethod(
            (kernel) =>
            {
                var resource = resourceResolver(kernel);
                var result = factoryMethod(kernel, resource);
                kernel.ReleaseComponent(resource);
                return result;
            });
    }

A bit of a mouthful, but then using it would look like:

Component.For<MyComponentType>()
    .UsingFactoryMethod(
         k => k.Resolve<SomeAlternativeComponent>(),
        (k, alt) => (new ImplementingComponentType(alt.RandomProperty)));

This reduces the amount of code to write a bit more, and it helps remind people to use it by being on their intellisense. I'd say it's somewhat a matter of judgement whether this is more or less readable than the helper method version. It is also less flexible if, for example, you need multiple different explicitly resolved resources inside a single factory method. But, as far as I can think, this kind of approach is likely to be the only fruitful way to achieve what you want.

UPDATE

Another alternative using the decorator pattern, which allows you to deal with multiple explictly resolved services by decorating the kernel:

public static ComponentRegistration<TService> UsingReleasingFactoryMethod<TService, TImpl>(
    this ComponentRegistration<TService> component, Func<IKernel, TImpl> factoryMethod)
    where TService : class
    where TImpl : TService
{
    Converter<IKernel, TImpl> safeMethod = (kernel) =>
    {
        var releasingKernel = new ReleasingKernel(kernel);
        var result = factoryMethod(kernel);
        releasingKernel.ReleaseAll();
        return result;
    };
    return component.UsingFactoryMethod(safeMethod);
}

Here ReleasingKernel is a decorator for Kernel which keeps track of all services resolved, and otherwise passes all calls through to the kernel. It also has a method which I'm calling ReleaseAll which calls to the contained kernel to release all services it has recorded.

This doesn't actually have to be a decorator, you could create your own class which only has a subset of the interface of IKernel available, if not all methods are needed.

So using this, your example would look like:

Component.For<MyComponentType>()
    .UsingReleasingFactoryMethod(k => {
        var altComponent = k.Resolve<SomeAlternativeComponent>();
        return new ImplementingComponentType(altComponent.RandomProperty);
    })

Definitely shorter and more readable. In terms of people remembering to use it, it will appear on the intellisense, but with a different name to UsingFactoryMethod.

\$\endgroup\$
2
  • \$\begingroup\$ Like you say, braces come down to preference, and I just find it easier to read with them on the starting line, I would put comments on there, just left out for brevities sake. I did think of the delegate approach, but there are instances where we're resolving multiple services at once, so it doesn't really help there. To my mind the main benefit is being able to wrap and return from the using block, rather than assigning the resolution locally, assigning the instance, and then releasing the resolved before returning the instance. \$\endgroup\$
    – Psytronic
    Jul 19, 2014 at 18:59
  • \$\begingroup\$ @Psytronic Updated with another possibility, which can handle multiple services. I think I like this one much better \$\endgroup\$ Jul 19, 2014 at 19:41

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.