(Warning: really boring post follows. This is what’s been on my mind today, but Jordan will probably wish that I would go back to talking about Java.)
There’s this big monster class that I’ve been dealing with at work almost ever since I got there. About a month or two into my job, I had to try to write unit tests for it, and failed miserably. (Because, after all, it’s a big monster class, exactly the sort of class for which the notion of “unit test” is ridiculous.) I did a bit of refactoring at the time, but for various reasons (I wasn’t very good at refactoring, and I wasn’t in charge of the code), it thoroughly repulsed my efforts at civilizing it. I did get a bee in my bonnet that State might be a useful design pattern to use, though.
A year later, I’m now in charge of the code, and it’s been causing me a fair amount of pain (in the form of seg faults, and sleepless nights about the thought of having to add new functionality, as I’ll have to do over the next few months). It actually was a pair of messy classes; I spent January matching wits with the first one. It certainly fought back – near the beginning, for example, I thought I had a nice bit that made sense conceptually to extract as a separate class, and I found a couple of method that looked like a perfect entry point into that section of code. But after spending a few days trying to get that to work, it just got worse and worse: those two functions called functions that called functions that referred to data that I thought was outside of the class in question, and it was really hard to tangle the code apart. So I had to throw away those days of work, and try again. (The second time was the charm, though.) And, actually, while that code is hugely better now than it was, I still wasn’t able to properly tame and test some of the core algorithms…
So this month I’ve been dealing with the other one of these messy classes. At the start, finding refactorings to do was like shooting fish in a barrel – anybody who can’t find methods to extract from 100-line functions isn’t looking very hard. (Let’s start by having every method fit on a single screen…) So far, I’ve extracted three nice little classes from it: they’re quite coherent, much easier to understand, much better tested, and I of course found several bugs in the process. One of the extracted classes, in particular, was an absolute joy to refactor: I knew that I had extracted a coherent chunk of data and methods, but I was having the hardest time figuring out what it was actually doing. This made adding unit tests a surprising pain; I ended up stopping thinking about what the class should do, and just mechanically writing tests that pinned down the class’s behavior, without worrying about how to interpret that behavior. (Well, almost pinned it down – fortunately, I kept around an end-to-end test as a backup, which saved me at one point in my later refactoring.) And when I got to the refactoring proper, I decided to do it strictly by the book, making really small, mindless little changes, and consciously trying to avoid look to far ahead, never making a jump when I could find baby steps that would get me there instead. And it was beautiful: the badness just melted away, the code almost transformed itself, and at the end of the day, everything made sense, and was clear to anybody reading it. I’m completely sold: tiny refactorings for me from now on.
But a week or so ago, I ran out of obvious classes to extract, so I decided to do the State extraction that I’d had on my brain for the last year and a half. I did it (by tiny steps), but while I liked the code more that way, I was pretty sure that most of my coworkers, when reading the code, would find it less clear. So I didn’t check it in: I wanted to spend more time moving functionality into the new State objects, and hoping that other refactorings would become obvious to me as I did so, so that by the time I checked it in the code would be clearly improved.
A couple of days later (I started a week ago, but most of this past week I was at a conference) I’ve got a lot more functionality moved into the State objects, but it’s still not a clear improvement. As I hoped, though, I am starting to see other refactorings that make sense, that really do improve the code.
The thing is, though, these other refactorings don’t depend in any way on my State extraction: in fact, if I hadn’t had this State bee in my bonnet, I would probably have seen them a week ago. Look at all these methods that take 6 arguments – maybe I could get rid of some of them? Look at these three member variables whose names all start with “uncopied”, with a big comment before them explaining how they’re used together – maybe I could extract those all into a class? And, as I do the extraction, I find what must be a bug in the code, but I’m so deep into refactoring upon refactoring that I have a hard time stepping back, figuring out what’s going on, and writing a test to expose the bug.
So I’ll be throwing away the last three days of programming: starting over, doing the obvious refactorings that I came across today, and probably creating a big parser wrapper class whose State I will only extract after I’ve gotten it into a coherent whole. Sigh.
Which, actually, isn’t so bad. At least I can say that I’ve learned something about programming over the last couple of years: I’ve learned that I should be nervous if I want to go for three days without checking in my code, and that it’s better to throw away that work and start over from scratch than keep on forging into the muck. And I’m sure that I’ll hit the ground running next week: having struggled with the code today, I’ll be able to do the first few refactorings in a flash, and it will only take me a day and a half to recreate the work that I’ve done in the last three days. (But there will probably be another week of work inserted into the middle of that day and a half of recap, as I do more refactoring to get it into shape before doing the State extraction.)
The next post will be a non-programming one, I promise…
Post Revisions:
There are no revisions for this post.