What to look for when reviewing code for SOLID principle ‘violations’

If your tasked with doing a code review of a fellow developers code or indeed are looking to improve an existing code base as a whole, one good set of principles to review the code by is the SOLID principles by Uncle Bob.

There are five SOLID principles in total and these courtesy of Wikipedia are:


Here’s what you should keep an eye out for if looking for violations of these principles:

Single responsibility principle – look for large classes which contain lots of functionality, chances are those classes could be broken up into smaller more focused ones. When your dealing with already small size classes the line is a little less clear as really what is the definition of ‘single’.

Open/closed principle – look for long blocks of if/else statements which check an object’s type and then does some similar action X (but differently) based on the result. When you add a new relevant type the class with this if/else type check code has to be modified to accommodate the new type, whereas the better way would be to use polymorphism either through interfaces or abstract classes.

Liskov substitution principle – this one may be a little tougher to spot and indeed is it often the toughest principle for developers to grasp. Look for subclass methods which change the behaviour (even if only sometimes) of a base class method in such a way that consumers can’t call the derived method and have it ALWAYS behave as if it was the base class method. To find LSP violations look for things like:

Subclasses with overridden methods which throw NotImplementedException().

Subclass methods which enforce stricter rules on parameters than their base.  For example… a base method accepts an integer as parameter but overridden method throws exception if this integer is not positive.

Methods which appear from a signature point of view to operate on a base class (or interface) but then within the method some type checking occurs. 

Interface segregation principle – look for large (fat) interfaces with a lot of unfocused methods, also look at interfaces whose clients throw NotImplementedException() a lot.

Dependency inversion principle – look for a lot of new statements.  New statements means concrete instantiations, which means coupling. Better to use a dependency injection framework or even (if you must) poor man’s dependency injection and push the creation of your objects up higher in the stack while simply passing down interfaces which provide a functionality contract but which makes no assertions about how that functionality is implemented.

Related Links

Overview of the five different principles. I always seem to be reading this article come interview time.

Organise ASP.MVC projects by feature by changing view location in MVC

I think it would be a safe bet to say most people stick with the default ASP.Net MVC folder structure and set up whereby the project structure is organised by architectural role (e.g. models, views and controllers) and not by business feature (e.g. registration, shopping cart, order history etc. ). For small projects to me this is fine. For bigger projects however it can be cumbersome to navigate as code related to particular features are scattered over multiple folders.

This ‘ by architectural role’ organisation goes against how in my experience software is written. Usually we build systems out feature by feature rather than writing all HTML markup, all controllers, all model classes etc. During development, this is particularly true in teams that embrace agile with its focus on delivering something that works and adds value at the end of each sprint. Imagine spending a three week sprint just writing viewModels… business value derived at end of sprint = 0. Check out this post on stackoverflow for some interesting comments on the merits of organising MVC by feature.

I’m sold… how do I organise my ASP.Net MVC project folders by feature? Well, given that MVC searches for controllers by type and name (not folder location) you can simply just start creating folders for ‘Registration’, ‘CheckOut’ etc. in the root and put your controllers, viewmodels and models in them and MVC will work fine. Views however are required to be in one of a number of predefined locations so simply creating folders won’t help you here. MVC is very extensible so we can of course change view locations and I will outline all the steps required to do this later. First however let’s look at MVC built in ‘Areas’ to see if they can meet our requirements ‘out of the box’.

Organise MVC projects using areas

Areas are around since MVC 2 and are logical groupings of controllers, models and views and other related folders (such as images, JavaScript etc.) for a particular module or feature in an MVC application.

mvcAREASA downside of areas unfortunately is that within them things are still organised into three folders; models, views and controllers. Yes everything within an area is related to a particular feature (a big step in the right direction) but code within that feature is still scattered as can be seen from the image to the left.

If this project layout gives you what you need, excellent, areas is what you should use. The extra advantage of using areas is of course they are available out of the box so if you have problems with them there is likely to be a lot of help available online. Snesh Prajapati’s article on areas in MVC 4 (no major changes in MVC 5 (not sure about MVC 6) except for how they are added I believe) on codeproject.com is one of the better articles on areas. In her article she notes the downsides of just creating moduleA, moduleB etc. folders under each of the root controllers, models and views folder. She also outlines the routing related code needed to wire up areas.


For my recent projects areas didn’t meet my needs as I was aiming for a structure similar to the right whereby everything could be in one folder. I know I could get close to this by just putting controllers, models and viewmodels in an area root and deleting the controllers and models folders. This however means my URLs get longer (a consequence of using areas), linking between areas becomes a little more ‘complicated’ and of course I’m still forced to put my views in the views folder (albeit a feature specific one).

Steps required to change view location in MVC

Areas don’t check all my checkboxes so I needed to find out how to change the location of where MVC looks for views. If you’re interested in doing this yourself, note there are a number of steps involved (overriding the default viewEngine is not enough!!!) but all are pretty easy to follow. The steps to change view location in MVC are:

Clear existing viewEngines, add back RazorViewModel, set new locations

The first step to change view location in MVC is to remove existing view engines and add RazorViewModel back in and define where the views are. Below I’m instructing MVC to look in ~/Shared/Views/VIEWNAME.cshtml (which will be the new location for shared views) and ~/CONTROLLERNAME/VIEWNAME.cshtml for my normal views. After this MVC does not care about the default ~/Views folder anymore. Note to change view location in MVC you don’t have to create a new ViewEngine like many places recommend.

Set the new view paths in Application_Start to change view location in MVC


Copy old ~/Views/web.config into ~/web.config so views will work correctly

Merge the ~/Views/web.config file with into the root web.config file. Views derive from System.Web.Mvc.WebViewPage and the root web.config doesn’t have this assembly referenced by default. Referencing it from root web.config means you don’t have to add a web.config for each new feature folder you add. Not everything needs to be brought across as some of it is there already, just focus on sections highlighted in yellow below.



Edit handlers in ~/web.config to explicitly allow JS, CSS etc. to be served

Set BlockViewHandler path to ‘*.cshtml‘ in the now updated root web.config. The web.config in the views folder had this set to ‘*’, meaning IIS would, for security reasons refuse to serve any files from the views folder. Setting it to .cshtml means all the CSS and JS will render correctly…. but wait…..

cshtml blocked

… note this is the blacklist approach where we are explicitly blocking .cshtml files from being served. We may add a new type of file in the future to a location in the project that doesn’t have it’s own web.config (and hence uses root web.config) which we do not want served but if we forget to change the above it will be silently served.

A more secure approach is to whitelist and explicitly allow whatever extension we know to be safe to be served. I’m obviously recommending the whitelist approach but I’ve included the blacklist approach just to illustrate the differences between the two as not many articles discussing how to change view location in MVC mention this when they advise the whitelist approach.



Move views to new shared views folder, update _ViewStart.cshtml and tidy up

Copy all files ~/Views/Shared to ~/Shared/Views. The original views/shared folder is now a dead duck, bring whatever views (most likely _Layout.cshtml at least) you need from there to the Shared/Views folder which hangs off the root.

Copy _ViewStart.cshtml from ~/Views to ~ and update the layout path inside it to point to new location of the _Layout.cshtml file.

new viewstart

Delete existing ~/Views folder as we have everything we need from it now.

Delete controllers and models folder in root when you moved all your classes to their own feature folders.

Related Links

The above relates to MVC 4 and MVC 5. MVC 6 has not been released yet, it’s still in RC1 and RC2 is not yet scheduled so it could even be 2017 before we see a final RTM version. If you’re playing with RC1 and looking to know how to do change view locations with it, the following two links may help:

Customize ASP.NET MVC 6 View Location Easily
What’s new in MVC 6: View Location Expanders


matthewFor an absolutely great overview of ‘Clean Architecture‘ which is the underlying pattern behind the idea of organising software around business features please check out Matthew Renze’s excellent video on Youtube.