Tuesday, October 30, 2012

Legacy App Nightmare

I'm working on a project for work that includes an update to a legacy application.  The application was written by contractors about six years ago, and it appears that they weren't given any time or incentive to refactor any of their work.

The legacy app is a Windows form application written using C#.  It probably had a decent design to start - some of the framework of the application makes it easy to add some new functionality to the application.  Other decisions they made are completely perplexing to me, and unfortunately their isn't anyone around to ask questions to of why things were implemented the way they were.

I had to start digging - stepping through the code, and making notes along the way.  One thing that occurred to me is that I never want someone to look at code I've written and feel as frustrated as I've felt when slogging through this particular legacy application code.  I feel like I follow good coding practices, but it is nice to have opportunities to see what things are like when good practices are not followed.

1. Comments are not bad - even in today's "agile" world.  Good coding principles dictate that we should name methods so that we know what the method is doing, but how do we tell people why we implemented the method the way we did it?  We can use comments!  If there is no ambiguity as to why a method is doing what it is doing, then no comment is necessary.  Just remember to consider whether or not the method will make sense to someone that was not involved in the original design when they look at it six years later.

2. Use informative and accurate method names.  This is not a new idea, but it doesn't mean that it is always easy or that it isn't overlooked.  The code I need to modify has a method called ProcessList.  That's great.  If I were to see that in a call stack, then how would I have any idea of what was being done?  I now need to look at the method to see if the contents in the list are being modified, if the contents in the list are keys for some other action that will work on some other bit of data, if the list is being modified, or something else.  Another thing to watch out for when it comes to method names is that your design might shift a bit, and your initial method names might not be quite as informative as they initially were.

3. Use informative variable names.  It is incredibly frustrating to see methods that have variables like this:

List<int> list = new List<int>();

I want to know what the list is being used for.  I don't want to just know that something is a list of ints.  The name might help you recognize a non-fatal logic problem in the code.

Refactoring is important to do on your project prior to releasing to production.  If you wait to refactor as a followup project, then there is a good chance that the time to do the refactoring will never be available.  It makes sense that the time won't become available, because the cost for the refactoring is potentially higher in a followup project/story than it is if you refactor as part of your original project/story.  The reason it might be more expensive is that you could be taking resources away from other projects that could be generating new revenue.  Also, it doesn't take too much time to go by before non-refactored code can become confusing if the why's aren't commented, and the method names are poorly chosen, etc.