Now this code has a fairly simple purpose, but it turns out to be really quite complex. For one thing, part of its functionality is implemented as a Windows device driver, which is something you've not worked on before.
For another thing, before Abby looked after it it was worked on by at least three other programmers and it looks like they all thought it was ugly, so it didn't matter if they made it a little bit more ugly. (Like Alan J. Perlis said, "In the long run every program becomes rococco - then rubble.")
So anyway, you start trying to figure out how to make the little change and it gets curiouser and curiouser, and some of the code is like "WHY did she do THAT?" and some of the comments are unhelpful, if not mocking. And the thing you thought would be bound to be little turns out to be spread through the code in Windows Resource files, ATL template parameters, hard coded string literals, on-the-fly generated names, ...
Anyway, it turns out to be harder than you thought. In fact you don't really understand the code. It works right now. But you have to change it in a way that was not anticipated when it was designed, so it's not easy, so you really need to understand the code before you change it. Plus, being a device driver, if you break it it has the potential to blue-screen thousands of your customer's computers.
Sadly, Abby doesn't work for the company any more, so you can't even talk to her about it. (Actually, if she did still work there it would be her responsibility to make the change, which would be even better.)
Here is a small dilemma I sometimes face at a time like this: the style of the code is bizarre and you can't tell what it's doing and the comments don't help; if you work through the code tidying it up - changing the indenting, brace-placement, removing the seemingly random blank lines, adding comments that make sense, removing comments that don't, simplifying unnecessarily complex code - you start to own the code, to understand the code. But if you do all this and you break the code, the diff against the previous version is just going to show massive changes. The material changes to the logic and functionality will be drowned in all the cosmetic changes. Finding what you broke will be hard.
Remember, this is a Windows device driver; it's not a small function that has 200 unit tests already written. (I'm sure there could be some kind of automated tests, but in the long history of this code no one has made any.)
My gut feel is to leave it alone this time. I'm only making a little change after all. The more I come back here, the more I'm going to "own" the code, and the more confident I'll feel about making big changes.
I read an interview with Bernie Cosell ("one of the fathers of the internet"), in Peter Seibel's great book Coders at Work, who habitually takes the more courageous path:
As I mentioned before, there were many bugs that I never had any clue where they were. I just get to a point where I say, "This piece of code is supposed to be doing this. This does not look like it's doing that. I mean, how could anybody have written this complicated bit of code to do this simple thing?" So I'd rip it out and replace it with a routine that does the simple thing I thought that piece of code was supposed to do. And the program magically works. In retrospect, what had happened is that the program had evolved and this little routine kept getting changed. Rather than being replaced when the program evolved, somebody was patching it to do different things and missed once.
I think this is all fine and dandy, so long as the guy that does the rewrite is still around to deal with the consequences, should there be any. I think I'd find it pretty irritating if someone rewrote my code, even though they didn't fully understand it, and broke it, and I had to work out what went wrong, and the diffs were massive.
Not that that happens to me, because I don't write complex code. And if anything looks a bit odd I write a comment explaining why it needs to be that way. So there.
No comments:
Post a Comment