Refactor: now = immediately


I was talking to a business colleague who stated that it is nice to refactor immediately, but the reality of business is you do not always have the time. I agree with the idea on the surface, but there are so few refactorings that should wait until some later mystery date.

My point in case is the project I am working on now. Built two years ago and partially refactored. We had one group coding in VB and another in C#. While this works, even in a .NET 2.0 website, it is not an optimal situation for so many reasons. I did complete most of the refactor (operation Kill VB) about a year and a half ago.

Today, we are moving sites from one collocation facility to another. I am now finding what a chore it is, largely because we left a lot of bad code in the site. Everything is in the config, but it is a royal mess. And there is precious little documentation on the system.

Now, I have heard Corey Haines talking about comments being evil, and I agree with his premise. If you need comments, your code is not clear. I also think you can reduce the amount of technical documentation with a quick refactor. You still need to write some documentation, but you can greatly simplify things.

As it stands in this project, there are three different connection strings, all pointed to the same database. One extra connection string was necessary, as certain elements were coded using .NET Tiers in CodeSmith. Sure, I could have whacked at the classes in .NET Tiers, or better yet switched all of my connection strings to the same name, but it was not done at the time.

Here are a few rules I think you should stick by firmly:

  • Always remove dead code as soon as it becomes dead. Dead code not only affects performance, especially with JITting, but it also leaves points of confusion in the code.
  • Configuration files are code files, so the above rule applies to them, as well. There is nothing like searching through a config with tons of garbage used for nothing.
  • Get rid of globals, except where absolutely needed. In most cases, wrapping them in a Singleton makes the most sense, if you need something global.
  • Learn to feed methods. Assume they are dumb. This is one surefire way to get rid of globals in a class (module level variables). When you pull from outside the method, you create a potential point of error.
  • Reread the last rule and make sure you NEVER do this across class libraries. One of the things that absolutely pisses me off is when a library I have referenced pulls directly from the web config file. I can allow a bit of leeway if it is a web control, but having some vendor library yanking a mystery variable out of thin air is very hard to debug.
  • Always eliminate duplicate code. This is the most common and also the stinkiest of the code smells. If you are using the same loop, branch, algorithm, etc. more than once, move it to its own method.
  • Get your code out of your ASPX pages. And, no App_Code is not an acceptable dumping ground. The ASP.NET “application” is a user interface. The pages, and classes, within should not be calculating user discounts. If you have to do this because you are used to it, make sure you leave time to undo it when you go to creating your test build. This pays off in dividends in numerous ways, including a) it is unit testable and b) you can swap out the UI for the latest Silverlight build without a huge amount of work.
  • Warnings are errors. This one is overlooked all the time, but it is critical to get rid of all warnings, as they are signs of weaknesses in your code. If you find an exception, don’t fret it. Rules are meant to be broken some times, but if you have more than a small handful of warnings, you are making WAY TOO MANY exceptions.
  • Be consistent in how you set up files, including config files.  You need to be able to find stuff. ‘Nuff said.

I am sure there are others, but these were the most glaring on this project. And, yes, some of them were my fault. I am a reformed non-refactor coder … and I have a ruler for your knuckles.

Hope you can use some of this today.

Peace and Grace,
Greg

Advertisements

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s

%d bloggers like this: