Developers want to write simple and elegant code. I do too, and so sometimes I catch myself being a little bit of a “refactoring bigot”, that when I came across code that I deemed not good, I became critical, or I just wanted to fix it all at once. Not very smart.
So here are a few things to consider, to avoid being a “refactoring bigot”:
Don’t criticize code by your team members
Code that is already written in your project, was written under constraints that you might not know about. It could have been a critical time pressure to finish the feature at hand. Or there might have already been a discussion between two developers pairing on it, and they reached a comprise based on other technical considerations. Or merely a difference of opinion: what I think will be prettier code might have its downfalls too. So, just like they say in Retrospectives: “… that everyone did the best job they could, given… the situation at hand.”
Do refactor, but do red-green-refactor
Don’t criticize, but do improve the code base when you see fit. Do the refactoring, but don’t be tempted to just go ahead and re-organize everything, and deviate from red-green-refactor cycle. You might come across a piece of code that you want to refactor while you’re trying to make a unit test pass. Make a note of it, make the test pass first, and then come back to do the refactoring. That way you can have the confidence that the tests will catch any errors during your refactoring. And the developer who’s pairing with you will be more engaged too, when there’s a clear boundary of making a test pass, and refactoring.
Still refactor to help understand code, and write more tests if necessary
One reason to refactor is to help oneself understand the code better. Especially when you delve into a new part of the code base, you might find it hard to read, whereas your pair might already be very familiar with it. If there is room for simplification, refactor to make it easier for you and others to understand.
With dynamic languages like Ruby and Javascript, I haven’t been using a true IDE with refactoring support. So sometimes that means simple refactoring tasks like “Extract method” are a manual process. Here I sometimes diverge from classic refactoring (keep tests unchanged, change the code, and have all tests still pass), and write an extra test, say, for the extracted method, or the newly created class.
This also break down the refactoring process into smaller manageable chunks, and having smaller chunks help your pair to see where you’re heading too.
Conclusion
Don’t be a refactoring bigot: don’t criticize, red-green-refactor, keep your pair engaged, keep refactoring in smaller chunks.
