Misuse of inheritance in web controllers

Inheritance is a common feature in OOP languages to specialise classes/types and allows polymorphism. It is easy to reuse the common functionality between classes by moving it to the parent class and use by the child classes. Without being mindful of software design principles, it can lead to increased coupling and responsibilities, making existing code hard to change. We’ll take you through a situation in C# MVC’s controllers and show how it improves the design by refactoring. The class diagram shown illustrates a fictitious online photo stock website.

This is a violation of interface segregation principle (ISP) which states that no client should be forced to depend on methods it does not use. In this case, BaseController has multiple methods and through inheritance, all of the child controllers have access to them but only use some of them and does not need some of them. When there’s a change in BaseController’s GetCredit, the child controllers which use it might be affected and should be checked and tested.

In Practical Object Oriented Design in Ruby page 112, inheritance is suitable for “highly related types that share common behaviour but differ along some dimension”. In the examples, there was a Bicycle which MountainBike and RoadBike inherited.

In addition, in terms of single responsibility, the controllers’ responsibility is to handle incoming requests, perhaps validate, send messages to domain objects, show views and redirect. It is not the controllers’ responsibility to implement methods like GetCredit and GetPaymentMethods.

Let’s look at this as an example:

As a controller, BaseController does not need to know about GetCredit() and GetCredit() is not doing much other than calling AccountRepo. As such, the method can be inlined to the respective controllers.

First, using Resharper, let’s inline the variable accountRepo using Inline Variable (Ctrl R+I).

Result:

Looking at PaymentController which uses BaseController.GetCredit():

We can inline the method:

Result:

Similar to the above, the other methods like GetAccount(), EntitledToDiscount(), GetPaymentMethods(), AddPaymentMethod() and DeductCredit() were inlined. After refactoring, this is the class diagram:

There is reduced coupling because the child controllers depend on less methods in BaseController. As a result, the individual controllers are easier to change and test. BaseController still has one method GetSession() which the child controllers use to get user id from session.

This was a simplified example of misuse of inheritance in controllers where coupling was low. When the code is more complex, more refactoring needs to be done. Hope this provides a starting point about how to undo misuse of inheritance.