My current focus is on stabilizing about a dozen applications which I recently inherited. By “stabilizing” I mean I am in the process of cleaning up, documenting and fixing somebody else’s code.
These applications have been in existence for years. They were first coded in Classic ASP, then rewritten in .NET 1.1 and finally ported over to 2.0. A review of source control history shows these applications were designed, written, loved, patched, hacked, semi-maintained, rewritten, loathed and abandoned (repeat) by dozens of developers with various coding styles and, I suspect, varying levels of sanity.
If you are the typically developer, you want to work on the latest and greatest new development and you don’t want someone else’s crumby code thrown onto your plate. You, frankly, believe you have better things to do and you squirm at the mere thought of “production support.”
But if you are me, you first squirm and then you get over it knowing it can’t be that bad…and then you crack open the code and quickly consider stabbing yourself in both eyes just to lessen the pain. If you are me, you are relieved in the fact that the application merely fails to connect to SQL rather than seeing what the code would do if it had a chance to execute. If you are me, you resolve one defect just to uncover two more and you seriously wonder why you are being punished as thoughts of Sisyphus run through your head.
But sometimes we have to suck it up and we have to do this unpleasant work. When you find yourself in my shoes, try to find the good in it. Acknowledge the fact that this is about as close to instant gratification with little risk as one can get in software development. Appreciate the fact that the damage has already been done (the code is already broken or really ugly) and you can only make things better (most likely very quickly too.) Know that there is a lot to be learned in observing what earlier developers did right, but also realize there is so much to learn from what other have done wrong. Finally, by all means, do everything you can to make things easier for the poor schmuck who will be cleaning up after you someday…
In the spirit of this post, here’s my most recent fix. What’s wrong with the following code? Please assume the AppActiveFlag column is a tinyint with a value of 0,1 or null:
bool isAppActive = false; System.Data.SqlClient.SqlDataReader dbReader = dbCommand.ExecuteReader(); if (dbReader.Read()) { object o = dbReader["AppActiveFlag"]; if (null == o) { isAppActive = false; } else { isAppActive = Convert.ToBoolean(Convert.ToInt32(o.ToString())); } }
If it helps, here’s the refactored logic:
bool isAppActive = false; System.Data.SqlClient.SqlDataReader dbReader = dbCommand.ExecuteReader(); if (dbReader.Read()) { if (!dbReader.IsDBNull(dbReader.GetOrdinal("AppActiveFlag"))) { isAppActive = Convert.ToBoolean(dbReader.GetByte(dbReader.GetOrdinal("AppActiveFlag"))); } }
LOL — very often the crummy old code I want to poke my eyes out over is code I wrote as little as two years ago that I’m just revisiting.
When the poor code is my own (and I have produced my fair share), as punishment, I tend to stab myself in the thigh with a fork rather than poke out my eyes.
You mean DBNull != null? Uh, oh.