How to Refactor to Generics from Class that Inherits from CollectionBase?

Question

I am working on an application that is about 250,000 lines of code. I'm currently the only developer working on this application that was originally built in .NET 1.1. Pervasive throughout is a class that inherits from CollectionBase. All database collections inherit from this class. I am considering refactoring to inherit from the generic collection List instead. Needless to say, Martin Fowler's Refactoring book has no suggestions. Should I attempt this refactor? If so, what is the best way to tackle this refactor?

And yes, there are unit tests throughout, but no QA team.

Answer 1

Don't. Unless you have a really good business justification for putting your code base through this exercise. What is the cost savings or revenue generated by your refactor? If I were your manager I would probably advise against it. Sorry.

Answer 2

250,000 Lines is alot to refactor, plus you should take into account several of the follow:

1. Do you have a QA department that will be able to QA the refactored code?
2. Do you have unit tests for the old code?
3. Is there a timeframe that is around the project, i.e. are you maintaining the code as users are finding bugs?

if you answered 1 and 2 no, I would first and foremost write unit tests for the existing code. Make them extensive and thorough. Once you have those in place, branch a version, and start refactoring. The unit tests should be able to help you refactor in the generics in correctly.

If 2 is yes, then just branch and start refactoring, relying on those unit tests.

A QA department would help a lot as well, since you can field them the new code to test.

And lastly, if clients/users are needing bugs fixed, fix them first.

Answer 3

I agree with Thomas.

I feel the question you should always ask yourself when refactoring is "What do I gain by doing this vs doing something else with my time?" The answer can be many things, from increasing maintainability to better performance, but it will always come at the expense of something else.

Without seeing the code it's hard for me to tell, but this sounds like a very bad situation to be refactoring in. Tests are good, but they aren't fool-proof. All it takes is for one of them to have a bad assumption, and your refactor could introduce a nasty bug. And with no QA to catch it, that would not be good.

I'm also personally a little leary of massive refactors like this. Cost me a job once. It was my first job outside of the government (which tends to be a little more forgiving, once you get 'tenure' it's damn hard to get fired) and I was the sole web programmer. I got a legacy ASP app that was poorly written dropped in my lap. My first priority was to get the darn thing refactored into something less...icky. My employer wanted the fires put out and nothing more. Six months later I was looking for work again :p Moral of this story: Check with your manager first before embarking on this.

Answer 4

I think refactoring and keeping your code up to date is a very important process to avoid code rot/smell. A lot of developers suffer from either being married to their code or just not confident enough in their unit tests to be able to rip things apart and clean it up and do it right.

If you don't take the time to clean it up and make the code better, you'll regret it in the long run because you have to maintain that code for many years to come, or whoever takes over the code will hate you. You said you have unit tests and you should be able to trust those tests to make sure that when you refactor the code it'll still work.

So I say do it, clean it up, make it beautiful. If you aren't confident that your unit tests can handle the refactor, write some more.

Answer 5

If you are going to gothrough with it, don't use List< T>. Instead, use System.Collections.ObjectModel.Collection< T>, which is more of a spirtual succesor to CollectionBase.

Answer 6

How exposed is CollectionBase from the inherited class?
Are there things that Generics could do better than CollectionBase?

I mean this class is heavily used, but it is only one class. Key to refactoring is not disturbing the program's status quo. The class should always maintain its contract with the outside world. If you can do this, it's not a quarter million lines of code you are refactoring, but maybe only 2500 (random guess, I have no idea how big this class is).

But if there is a lot of exposure from this class, you may have to instead treat that exposure as the contract and try and factor out the exposure.