There's a lot of posts and chatter about testing, TDD, and a number of other topics on this blog, the larger devlicio.us site, and the .NET community in general.  Before all that, it should go without saying, but it must be said; there is a pre-requisite to testing, care about your code.

You have probably come across some code in your career that is less than ideal.  Maybe you've even submitted something to the Daily WTF. I'm currently combing some old code trying pull out some main concepts.  As I'm going through the code I'm seeing some things that really are quite alarming.  I'm not talking about differences in what I would do in terms in architecture, implementation, or even language choice.  I'm talking about blatant code rot, code that no one has cared for.

Some stats for the current file I'm in:

• Lines in file: 1820.  This is the mildest offense, while I don't endorse a class/file that large I can live with it.
• Classes defined in the file: 19.  I find this a "little" excessive, Even if you don't subscribe to the "one class per file" rule, 19 is a bit much don't you think?
• Example Method: 697 lines.  That number isn't entirely accurate as 395 of the lines are commented out.  I understand the scenario that probably led to this, in fact I'll admit to doing it.  You comment out some code as a fail-safe in case the new implementation doesn't work.  The problem here is that the code does work (and I use that term loosely).  The developer never cleaned up after themselves and their trial code.  This makes absolutely no sense when you have source control in place.

Here's a method I came across today:

   1: Public Overrides Function ToString() As String

   2:     If IsNothing(_fields) Then

   3:         Return ""

   4:     End If

   5:  

   6:     Dim r2 As String

   7:     Dim retVal As StringBuilder = New StringBuilder("", _fields.Length * 2) 'make room for 1 char and 1 delimeter for each field

   8:     For Each f As Field In _fields

   9:         r2 = r2 & f.Prefix & f.Value & f.Postfix & ConfigurationSettings.AppSettings("ControllerFieldDelimeter")

  10:         retVal.Append(f.Prefix).Append(f.Value).Append(f.Postfix).Append(ConfigurationSettings.AppSettings("ControllerFieldDelimeter"))

  11:     Next

  12:  

  13:     r2 = r2 & "<EOT>"

  14:     retVal.Append("<EOT>")

  15:  

  16:     'Return retVal

  17:     Return retVal.ToString

  18: End Function

The first thing I noticed was the string concatenation in line 9.  The problem is the "_fields" (the collection over which this code is iterating) can be large.  So concatenation isn't the best choice.  While clearly not optimal, let's let that slide for a moment.  The very next line, line 10, there is some appending of seemingly the same data.  In the end r2, the concatenated value which was so expensive to generate, is never returned or used elsewhere.  All of those extra, wasteful concatenation calls are spent on worthless code.  The developer(s) who worked with this over time simply never cleaned up after themselves or cared for the code.

I'm amazed that code like what I'm looking at is found in code bases.  I probably shouldn't be surprised but I am.  Today I was posting my reactions on Twitter and a friend Rhys Campbell commented:

"395 of them are commented out".. delete them! do it! You know you want/need to

I think Rhys is right, I will fix the code as I see it.  The codebase is being phased out but that doesn't mean it shouldn't be cared for.  If I am caring for code that is going to be obsolete and replaced soon, how much more important do you suppose is it to care for your current code?  If any of the above sounded like your code, get with it, clean up and care for your code.