1

I have the following helper class which has public property _variableHandler. The property is public as I initially had visions of setting the property from the calling code before involving methods within the XAMLHelper class, but now doubt this is a smart approach. Mostly because I will need to call the class fairly often and not always update the value of _variableHandler, meaning things will get messy.

public class XAMLHelper
{
    public IVariableTypeHandler _variableHandler;

    public XAMLHelper()
    {
    }
}

I also have a factory which is used to provide the desired concrete instance of a VariableTypeHandler.

I am also using an IoC container (Unity) to provide a single instance of the XAMLHelper class, as shown below.

container.RegisterInstance<XAMLHelper>(new XAMLHelper());

Ideally, I would like to keep this single instance but simply update the value of _variableHandler when specified using like the code below.

container.Resolve(new PropertyInjection(VariableHandlerFactory.GetInstance("Str")));

I have tried adding snippets into container registration, such as a

new InjectionProperty()

But this doesn't seem to update the _variableHandler property once it has been instantiated the first time. Am I missing something important here? Or trying to do something that is not possible with an IoC container?

  • Does `VariableHandlerFactory.GetInstance("Str")` return the instance of the `IVariableTypeHandler`? Also I think you need to annotate property with `[Dependency]` attribute... – Johnny Jan 22 '19 at 20:44

1 Answers1

1

Generally, good design proposes making helper\manipulator\operation classes stateless. Here you clearly have a helper class which is stateful as you are able to set the state of the object through the property _variableHandler (not real property, it is more like a field).

What you could do is to make factory for IVariableHandler register that within the IoC and inject it into XAMLHelper. Then when invoking helper you just specify which handler to use and re-create it with factory. Factory could be a bit smarter to reuse already create object using some kind of caching.

public intefrace IVariableHandlerFactory
{
    IVariableHandler Get(string description);
}

public class VariableHandlerFactory : IVariableHandlerFactory
{
    private readonly IDictionary<string, IVariableHandler> _cache;

    public VariableHandlerFactory()
    {
        _cache = new Dictionary<string, IVariableHandler>();
    }

    public IVariableHandler Get(string description)
    {
        IVariableHandler handler; 

        if(_cache.TryGetValue(description, out handler))
        {
            return handler;         
        }

        handler = //create it...
        _cache[description] = handler;

        return handler;
    }
}

and then use that within XAMLHelper

public class XAMLHelper
{
    private readonly IVariableHandlerFactory _factory;

    public XAMLHelper(IVariableHandlerFactory factory)
    {
        _factory = factory;
    }

    public void HelpMe(string description)
    {
        var handler = _factory.Get(description);

        //do actual work using handler
    }
}

Let's put that aside, and discuss the real problem here. First, you are missing annotation of your property:

[Dependency]
public IVariableTypeHandler VariableHandler { get; set; }

then you could register the XAMLHelper using the InjectionProperty

container.RegisterType<VariableTypeHandlerImpl, IVariableTypeHandler>();

using(var lifetime = new ContainerControlledLifetimeManager())
    container.RegisterType<XAMLHelper>(lifetime, new InjectionProperty("VariableHandler");

or, if you have concrete instance of the IVariableTypeHandler you could use:

var variableHandler = VariableHandlerFactory.GetInstance("Str");

using(var lifetime = new ContainerControlledLifetimeManager())
    container.RegisterType<XAMLHelper>(lifetime, new InjectionProperty("VariableHandler", variableHandler);
Johnny
  • 8,939
  • 2
  • 28
  • 33
  • Indeed, `VariableHandlerFactory.GetInstance("Str")` does return an instance of `IVariableTypeHandler`. Can you suggest a simple way to make the `XAMLHelper` class stateless but still use the VariableHandlers produced by the factory? I have also added the 'Dependency' attribute as suggested and then used this code `container.Resolve(new PropertyOverride("VariableHandler", VariableHandlerFactory.GetInstance("Int")));` to update the property from the default VariableHandler (as dictated in the container registration), but it does not seem to update the property. Any suggestions? – unknownpresense Jan 22 '19 at 22:59
  • @unknownpresense For example, why `XAMLHelper` have to be singleton, could you create it every time and inject `IVariableTypeHandler`? How many variation do of `IVariableTypeHandler`, could you register all types of it? – Johnny Jan 23 '19 at 07:09
  • The `XAMLHelper` class is called around 100 times per cycle and the process can cycle up to 30 times in a run. The `VariableHandler` property is updated roughly 6 times within those 100 calls. I figured making a new instance every time would be a bit overkill. There are currently 20 variations of `IVariableHandler` but that could easily grow to double that, which is why I decided a factory would be appropriate. I am trying to code to a high standard (following SOLID, DRY etc) so any improvements would be highly appreciated :) – unknownpresense Jan 23 '19 at 09:51
  • 1
    @unknownpresense For `IVariableHandler` you could have factory or builder that is fine, maybe factory with underlaying cache so you create only once and store if it is possible. Then inject that factory in `XAMLHelper` and invoke methods on XAMLHelper by specifying which `IVariableHandler` to use... Is that possible? – Johnny Jan 23 '19 at 10:14
  • That is a great idea! A bonus question here is how would I cache the previously created `IVariableHandler`'s? I considered making them all singletons, could this work? Also I believe your suggestion of specifying which handler to use will solve my problem! I can then simply check the current handler on `XAMLHelper` is the same as the specified one and update if not :D – unknownpresense Jan 23 '19 at 11:45
  • 1
    @unknownpresense You can cache them as simple as `IDictionary`. The one in `XAMLHelper` is not really needed but could be also like L1, L2 caches... Should I wrap this also in the answer? – Johnny Jan 23 '19 at 14:30
  • If you wouldn't mind, that would help me massively! Thank you, appreciate the help :) – unknownpresense Jan 23 '19 at 15:10