-1

I have to write a Complex calculator logic which has 4 different components to be calculated brokerage, stockprice, admin charges & other charges. Each having a different logic and formulas.

So I decided to use Unity DI. I have a ContainerFactoryClass which resolves all classes which implements IChargeCalculator interface as shown below in the TotalAnnualCostCalculator constructor.

public class TotalAnnualCostCalculator
{
    private readonly IUnityContainer container;

   //Constructor
    public TotalAnnualCostCalculator()
    {
        container = ContainerFactory.InitializeContainer();
        ContainerFactory.SetupContainer(container);
    }
    public AnnualCostCharges CalculateTotalAnnualCost(Parameters product)
    {
          var calculators = container.ResolveAll<ICalculator>().ToList();
          // Invoke calcualtion method

           Parallel.ForEach(calculators, c =>
        {
            return c.CalculateAnnualCost(product);

        });
    }

}

Container Factory class:-

public static class ContainerFactory
{
    public static IUnityContainer Container { get; private set; }
    public static IUnityContainer InitializeContainer()
    {
        var container = new UnityContainer();

        RegisterDependencies(container);

        return container;
    }

    private static void RegisterDependencies(UnityContainer container)
    {
        container.RegisterType<ICalculatorStrategyFactory, CalculatorStrategyFactory>("Factory");

        container.RegisterType<IEffectiveAnnualCostCalculator, InvestmentManagementChargeCalculator>("IMCChargeCalculator",
            new InjectionConstructor(new ResolvedParameter<ICalculatorStrategyFactory>("Factory")));
        //container.RegisterType<IEffectiveAnnualCostCalculator, AdministrationChargeCalculator>("AdministrationChargeCalculator");
        container.RegisterType<IEffectiveAnnualCostCalculator, AdviceChargeCalculator>("AdviceChargeCalculator");
        container.RegisterType<IEffectiveAnnualCostCalculator, OtherChargeCalculator>("OtherChargeCalculator");

        container.RegisterType<IInvestmentManagementChargeCalculator, LumpSumIMCCalculator>("LumpSumIMCCalculator");
        container.RegisterType<IInvestmentManagementChargeCalculator, DebitOrderIMCCalculator>("DebitOrderIMCCalculator");
    }

    public static void SetupContainer(IUnityContainer container)
    {
        Container = container;
    }
}

Then any API consumes my Calculator.dll by just creating an instance of TotalAnnualCostCalculator and call a method like this.

var totalCharges = calc.CalculateTotalAnnualCost(prod);

My code reviewer says its better to use Factory Pattern ,as this tightly coupled to Unity Framework.

Please advise

Mark Seemann
  • 225,310
  • 48
  • 427
  • 736
  • first of all, Parallel.ForEach(calculators, c => { return c.CalculateAnnualCost(product); }); will only calculate one calculator (randomly depending on speed and position on the list) and return – Boo Jan 25 '18 at 13:23
  • If the goal is simply to de-couple from the DI framework then you can wrap all of the DI implementation in a simple custom container/interface that you control. Treat it like any other dependency that you abstract behind an interface. Basically write your own `IContainer` and public members of your initializer only accept/return that instead of `UnityContainer`. – David Jan 25 '18 at 13:23
  • 1
    Additionally, this may be more suited for https://codereview.stackexchange.com/ – David Jan 25 '18 at 13:24
  • 2
    Perhaps you should discuss this with your code reviewer. That said, consider using the Composite pattern instead of a Factory. – Steven Jan 25 '18 at 13:44
  • This question is really too broad. Suggest asking on CodeReview or SoftwareEngineering – Mark Benningfield Jan 25 '18 at 13:52
  • Starting with 'So I decided to use Unity DI' is a smell. – Sam Holder Jan 25 '18 at 14:10
  • If you have one IUnityContainer in ContainerFactory than you can resolve what ever you want from Container. This is service locator pattern and it is not good practice. Idea is to resolve only what you need in class, and container exist only on registration. – Raskolnikov Jan 25 '18 at 14:11
  • @MarkBenningfield when referring other sites, it is often helpful to point that [cross-posting is frowned upon](https://meta.stackexchange.com/tags/cross-posting/info) – gnat Jan 25 '18 at 16:33
  • @gnat: Good point. Thanks for the link. – Mark Benningfield Jan 25 '18 at 16:36

2 Answers2

3

Indeed, don't use a DI Container at all. As Steven suggests in the comments, this seems a great fit for a Composite:

public class TotalAnnualCostCalculator : ICalculator
{
    private readonly ICalculator[] calculators;

    public TotalAnnualCostCalculator(params ICalculator[] calculators)
    {
        this.calculators = calculators;
    }

    public AnnualCostCharges CalculateTotalAnnualCost(Parameters product)
    {
        Parallel.ForEach(this.calculators, c => c.CalculateAnnualCost(product));
    }
}

In the Composition Root, then, you simply new up all the ICalculator objects you'd like to use, and pass them to the constructor of TotalAnnualCostCalculator.

Mark Seemann
  • 225,310
  • 48
  • 427
  • 736
  • Thanks Mark. But this would be standalone library and other API's refer to this .dll . So why does the external users need to know all the concrete classes. Please can suggest for standalone library – shashi kumar Jan 25 '18 at 15:26
  • @shashikumar If the external users don't need to know about the concrete classes in use, then why use DI at all? In general, however, see https://stackoverflow.com/a/2047657/126014 – Mark Seemann Jan 25 '18 at 15:29
0

Register all IEffectiveAnnualCostCalculator (or what ever interface). You just need to map the enumerable to an array of the same type.

container.RegisterType<IEnumerable<IEffectiveAnnualCostCalculator>, IEffectiveAnnualCostCalculator[]>();

Resolve with dependency injection:

 private IEnumerable<IEffectiveAnnualCostCalculator> calculators;

 public TotalAnnualCostCalculator(IEnumerable<IEffectiveAnnualCostCalculator> calculators)
 {
     this.calculators = calculators;
 }

public AnnualCostCharges CalculateTotalAnnualCost(Parameters product)
{
    Parallel.ForEach(this.calculators, c => c.CalculateAnnualCost(product));
}
Raskolnikov
  • 3,791
  • 9
  • 43
  • 88