Question

I am continuing the development of an ASP.NET application (web form based) where the previous developer did not follow principles of good Object Oriented design i.e. SOLID (http://www.remondo.net/solid-principles-csharp-interface-segregation/). I have read posts like this: Long-held, incorrect programming assumptions.

The problem is that a lot of the classes have low cohesion and are highly coupled and a lot of the classes do not have a single responsibility (they have many). My specific question is: should I start following the SOLID principles or just continue developing by tweeking and adding more to the classes? I have always tried to follow principles like SOLID in the past, but the application I am talking about is very large and complex. I am the only developer working on this project.

A complete rewrite is out of the question at the moment but will be possible one day.

Update 15/07/2012 So far I have discovered that SOLID is a design principle and GRASP is a design pattern, which is perhaps more suitable to MVC rather than a Page Controller type application. Mock objects are also perhaps more suited to MVC according to this link: http://www.asp.net/mvc/tutorials/older-versions/overview/asp-net-mvc-overview. Based on the response so far it is always good practice to follow SOLID principles. However, the answers so far do not recommend design patterns for form based apps.

Was it helpful?

Solution

Try refactoring bit by bit.

If you are working on a module, try adding a few interfaces here and there ;-) as you go. Any new code that you write should be SOLID. Try encapsulating old code as much as possible, put old crap behind a facade. You don't have to rewrite it right now, maybe never. As long as you can encapsulate old code behind a facade and write unit tests for the facade you should feel a lot better.

Example: So lets say you have a collection of classes that implement some piece of functionality, ie. invoice processing. Right now I imagine those classes are used in many places and there is a lot of duplication going on. The code for calculating total invoice amount is copied on every page that displays it. What I would do in that case is to create an Invoice service which will expose methods for getting an invoice, adding new lines, calculating total amount, etc. You can than put the code that you have in this service without much refactoring, you will keep the same db schema and so on. But from now on your pages will interact only with IInvoiceService interface and you can mock it nicely. It is a bit of 'swiping dust under the carpet' but at least you can stop the crap spreading even further. Next time you need to do something in the invoicing module (fix a bug, implement new feature) you clean the existing code a bit. Over time the amount of dust under the carpet gets smaller and smaller. You just need to stick to your guns and make sure that the service interface stays nice and clean.

You may need to make some more system-wide changes, like introducing DI framework, but try keeping the changes to minimal. Usually when you start going to far you can easily find yourself rewriting everything, and you don't want that.

Make small changes, commit often.

OTHER TIPS

It's like asking "Well, I just bought an apartment from someone but there's dirt everywhere. Should I clean it up or just leave it as is?"

The answer is obvious. You said the application is complex. If it is complex just because of the bad architecture - there's your chance to make it more understandable. It it is complex because of the nature of the bussiness issue, the bad architecture just makes it worse.

SOLID are very important principles. Also pay attention to GRASP guidelines, I guess you do so because you mention low coupling and high cohesion and these belong to GRASP rather than to SOLID.

The most obvious approach is to refactor, step by step in a right direction. Decouple classes, define their responsibilities, make them more cohesive, introduce interfaces and code against them. These are much more important than SOLID's Open-Closed Principle or Liskov Substitution Principle.

My suggestion would be to pick a feature and start unit testing it and introduce a mock library such as Moq or FakeItEasy.

I completely understand this is a handed over code for you. But with unit testing you will come to know what code is a good candidate for Single Responsibility or must be a Service or must be moved in a Repository.

You are already going to implement DI, but also use something like AutoMapper saves you alot of time and keeps the code smaller and clearer

When I was handed over like your situation the very thing I did is read Uncle Bob,s book http://www.amazon.co.uk/Working-Effectively-Legacy-Robert-Martin/dp/0131177052 it gives you some good ideas

Update on your comment: The app after first 3 months have less bugs and a bit easier & quicker to implement new changes. The best part is that now we have the domain knowledge. One good thing that stopped legacy and new changes causing bugs was Anit-Corruption layer please read this link http://domaindrivendesign.org/library/peng_hu_2007_2

I've worked in a shop that had good unit test code coverage (over 3000 tests), which the writing thereof encouraged our code to generally follow the SOLID principles.

I've also worked in a shop with 0% code coverage (circa 0.5m lines of code), which didn't adopt SOLID in the slightest.

We couldn't refactor any of the half million lines of code (for SOLID or for any other reason) as we had no test coverage. So the code grew worse and worse to maintain over time.

We didn't need to refactor in my first job, as code was already SOLID, but we could have easily refactored and used the good unit test code coverage to ensure no regression.

It's like magic: to those who believe no proof is required; to those who don't believe no proof will ever be enough.

Let's put it like this: you want to work somewhere with code that is horrendously hard to understand, horrible to maintain, where devs leave as code is so poor, and where bugs are rampant or you prefer the opposite ? Code for the latter, always, whether SOLID or not, but IMHO SOLID with unit tests will make achieving this so much easier in the long run.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top