So I am a fan of dependency injection (DI), inversion of control (IoC), and the way DI and IoC allow for simplistic methods and Unit Tests. With DI, you can do method injection, property injection, or constructor injection. I don’t care which one a project uses, as long as they keep it simple.

Constructor Injection

This article is focussing on constructor injection. Constructor injection seems to be very popular, if not the most popular method of DI. Constructor Injection is considered to have a benefit because it requires the instantiator to provide all the dependencies an object needs in order to create an instance of it.

An Example of Constructor Injection Hell

Recently, I started working with NopCommerce, which uses DI heavily. They use Autofac and register objects with Autofac so it can provide concrete instances of any interfaces.

I am going to use NopCommerce as an example of what not to do. Now before I do this, I want to explain that NopCommerce overall has a very good architecture. Better than most. Finding something that I consider a “what not to do” in a project should not steer you away from NopCommerce. In fact, their plugin model and architecture works quite well.

Below is an example of constructor injection gone wrong from the OrderProcessingService.cs file in NopCommerce.

#region Ctor

        /// <summary>
        /// Ctor
        /// </summary>
        /// <param name="orderService">Order service</param>
        /// <param name="webHelper">Web helper</param>
        /// <param name="localizationService">Localization service</param>
        /// <param name="languageService">Language service</param>
        /// <param name="productService">Product service</param>
        /// <param name="paymentService">Payment service</param>
        /// <param name="logger">Logger</param>
        /// <param name="orderTotalCalculationService">Order total calculationservice</param>
        /// <param name="priceCalculationService">Price calculation service</param>
        /// <param name="priceFormatter">Price formatter</param>
        /// <param name="productAttributeParser">Product attribute parser</param>
        /// <param name="productAttributeFormatter">Product attribute formatter</param>
        /// <param name="giftCardService">Gift card service</param>
        /// <param name="shoppingCartService">Shopping cart service</param>
        /// <param name="checkoutAttributeFormatter">Checkout attribute service</param>
        /// <param name="shippingService">Shipping service</param>
        /// <param name="shipmentService">Shipment service</param>
        /// <param name="taxService">Tax service</param>
        /// <param name="customerService">Customer service</param>
        /// <param name="discountService">Discount service</param>
        /// <param name="encryptionService">Encryption service</param>
        /// <param name="workContext">Work context</param>
        /// <param name="workflowMessageService">Workflow message service</param>
        /// <param name="vendorService">Vendor service</param>
        /// <param name="customerActivityService">Customer activity service</param>
        /// <param name="currencyService">Currency service</param>
        /// <param name="affiliateService">Affiliate service</param>
        /// <param name="eventPublisher">Event published</param>
        /// <param name="pdfService">PDF service</param>
        /// <param name="rewardPointService">Reward point service</param>
        /// <param name="genericAttributeService">Generic attribute service</param>
        /// <param name="paymentSettings">Payment settings</param>
        /// <param name="shippingSettings">Shipping settings</param>
        /// <param name="rewardPointsSettings">Reward points settings</param>
        /// <param name="orderSettings">Order settings</param>
        /// <param name="taxSettings">Tax settings</param>
        /// <param name="localizationSettings">Localization settings</param>
        /// <param name="currencySettings">Currency settings</param>
        public OrderProcessingService(IOrderService orderService,
            IWebHelper webHelper,
            ILocalizationService localizationService,
            ILanguageService languageService,
            IProductService productService,
            IPaymentService paymentService,
            ILogger logger,
            IOrderTotalCalculationService orderTotalCalculationService,
            IPriceCalculationService priceCalculationService,
            IPriceFormatter priceFormatter,
            IProductAttributeParser productAttributeParser,
            IProductAttributeFormatter productAttributeFormatter,
            IGiftCardService giftCardService,
            IShoppingCartService shoppingCartService,
            ICheckoutAttributeFormatter checkoutAttributeFormatter,
            IShippingService shippingService,
            IShipmentService shipmentService,
            ITaxService taxService,
            ICustomerService customerService,
            IDiscountService discountService,
            IEncryptionService encryptionService,
            IWorkContext workContext,
            IWorkflowMessageService workflowMessageService,
            IVendorService vendorService,
            ICustomerActivityService customerActivityService,
            ICurrencyService currencyService,
            IAffiliateService affiliateService,
            IEventPublisher eventPublisher,
            IPdfService pdfService,
            IRewardPointService rewardPointService,
            IGenericAttributeService genericAttributeService,
            ICompanyService companyService,
            ShippingSettings shippingSettings,
            PaymentSettings paymentSettings,
            RewardPointsSettings rewardPointsSettings,
            OrderSettings orderSettings,
            TaxSettings taxSettings,
            LocalizationSettings localizationSettings,
            CurrencySettings currencySettings)
        {
            this._orderService = orderService;
            this._webHelper = webHelper;
            this._localizationService = localizationService;
            this._languageService = languageService;
            this._productService = productService;
            this._paymentService = paymentService;
            this._logger = logger;
            this._orderTotalCalculationService = orderTotalCalculationService;
            this._priceCalculationService = priceCalculationService;
            this._priceFormatter = priceFormatter;
            this._productAttributeParser = productAttributeParser;
            this._productAttributeFormatter = productAttributeFormatter;
            this._giftCardService = giftCardService;
            this._shoppingCartService = shoppingCartService;
            this._checkoutAttributeFormatter = checkoutAttributeFormatter;
            this._workContext = workContext;
            this._workflowMessageService = workflowMessageService;
            this._vendorService = vendorService;
            this._shippingService = shippingService;
            this._shipmentService = shipmentService;
            this._taxService = taxService;
            this._customerService = customerService;
            this._discountService = discountService;
            this._encryptionService = encryptionService;
            this._customerActivityService = customerActivityService;
            this._currencyService = currencyService;
            this._affiliateService = affiliateService;
            this._eventPublisher = eventPublisher;
            this._pdfService = pdfService;
            this._rewardPointService = rewardPointService;
            this._genericAttributeService = genericAttributeService;
            this._companyService = companyService;

            this._paymentSettings = paymentSettings;
            this._shippingSettings = shippingSettings;
            this._rewardPointsSettings = rewardPointsSettings;
            this._orderSettings = orderSettings;
            this._taxSettings = taxSettings;
            this._localizationSettings = localizationSettings;
            this._currencySettings = currencySettings;
        }

        #endregion

Problems in the Constructor Injection Implementation

So what is wrong with the above constructor? Well, a lot. Look, this is just bad code. While constructor injection is a good idea, taking it to this extreme is not a good idea. In fact, it is a terrible idea.

1. The Constructor has too many parameters. While there is no limit, there is a best practice. See this stack overflow post: How many parameters are too many?
2. The Constructor breaks the 10/100 rule. The constructor, with comments, method parameters, and method body is 126 lines of code. The method itself is far more than 10 lines of code, it is 39 lines of parameters and 39 more lines of member assignments, and is 80 lines of code.
3. The Constructor breaks the keep it super simple (KISS) principle. Having to new up 39 concrete instances of the parameters in order to create an object is not simple. Imagine mocking 39 interface parameters in a Unit Test. Ugh!
4. This constructor is a hint that the entire class is doing too much. The class is 3099 lines and clearly breaks the single responsibility principle. It is not the OrderProcessingService’s responsibility to store 39 dependent services.
5. The constructor breaks the Don’t Repeat Yourself (DRY) principle. Almost all other classes in NopCommerce use constructor injection to access services.

Options for Refactoring

Option 1 – Container object

You could create a container that has all of these dependecies, a dependency model object for the OrderProcessingService. This object would house the 39 dependent services and settings. But Option 2 would be better.

Option 2 – Accessor objects

Looking at this from the Single Responsibility Principle, shouldn’t there be one class and interface, a ServiceAccessor : IServiceAccessor that allows one to access any dependent service? Instead of passing in 30 services, wouldn’t it make more sense to pass in a single object called a ServiceAccessor that implements IServiceAccessor? Should there be a ServiceAccessor of some sort? Turns out there is a static: EngineContext.Current.Resolve(). Since it is a static, maybe you could wrap it in a ServiceAccessor : IServiceAccessor object.

There are also a lot of “settings” objects passed into the constructor? Shouldn’t there be a SettingsService? Well, there is. One can pass in the ISettingsService and then call _settingService.LoadSetting().

Instead of passing in 39 parameters, methods with a single responsibility to fetch a service should be used.

Option 3 – Refactor the class

Since the class is 3099 lines. If the class were broken into logical pieces, naturally, the constructor for each smaller piece would have less parameters.