Ricky says “don’t be a refactoring bigot”. And I have to say, his very first point makes no sense to me. He says “don’t criticize code”, especially if it is already in your project. I say, be a jerk. A big fat jerk. A “your code is a ghetto” style jerk. And this is ESPECIALLY true for code that is already in your project.
Refactoring is itself a kind of criticism: criticism through action. When you say “this code could be more elegant”, you are implicitly saying that it is not as elegant as it could be. If you follow this advice to its logical conclusion, you would never refactor the parts of your codebase that need it the most, the parts written by team members under duress, without proper pairing and review, which load up your technical debt.
As I said a couple of weeks ago, if you have something to say, don’t be afraid to say it. Growth comes from understanding; you can’t help people understand a problem if you ignore it. As a person who has been told several times in the last week that my code could use some, shall we say, “up-elegantizing”, I can say without fear of contradiction that that criticism is the only thing that would have convinced me to try and up my game.
So, yeah, don’t just refactor every piece of bad code you see just for the sake of doing it. But by all means, when you ARE going to do it, be a jerk about it. Shorten the feedback loop. Don’t dance around the issue. Your pair will appreciate the honesty, and your project will be better off as a result.
Comments
And, conversely, don’t be an oversensitive crybaby when people deign to improve your code.
Honesty != Being a Jerk
There’s no reason you can’t be polite about it. I think one of the few things more important than test coverage and clean code, is team dynamics.
I used to think that how I say something shouldn’t matter, as it is what I say that is important. I realize now that I was dead wrong. But it’s hard to explain why. So, let’s take a software development approach to it.
Why do we write (or refactor to) clean, readable, quality code? Why do we value Ruby’s expressiveness? As software developers, we value how the code is written, not just how the code executes. Ultimately we care because we’re writing the code not just for the computer, but also for other developers to read and understand.
Why should we disregard “the how” when programming in the language of humans? When programming the emotions of the person we’re talking to?
Emotions are the foundation of the points we’re trying to deliver, and the relationships we’re trying to build. Just like a clean code base with test coverage is necessary (or at least preferred) to extend new features into the software.
In a sense, our communication style – the how – is our code. The point we’re trying to deliver is just data.
Right now, far too many software developers have bad human emotion coding practices.
Cheers, Clinton
On of the first [lessons I remember listening too at school] about writing stories is that you write the story, and then you must expect to rewrite some/all of it. You assume that rewriting is part of the complete process of writing.
But I’m sure when I was 10 I didn’t have the same analysis of the lesson and I resented writing at all knowing the above to be true, but hating the thought of having to do the work more than once.
So with readable code its the same thing. You write it once (say to pass the tests) and you assume that you’ll rewrite it. Its even part of the TDD slogan “red-green-refactor”, not that a slogan is a justification for doing something, I guess.
But the lessons on writing stories and assuming you’ll need to rewrite them seems a useful analogy.
If code comes to me it is my code now. If I change it, too bad. If you want to know why then ask. If you don’t like it, too bad. Show me a better way.
Like Ricky said, there could have been any number of reasons it is bad, which is a good reason not to criticize the programmers, or make meaningless public gripes about the code which amounts to the same thing. Criticizing programmers will rarely come to any good unless it’s so bad that some sort of management action is necessary.
However, that’s not the same as criticizing the code itself! Being able to see what’s wrong with code, explain it, and fix it is a necessary part of any senior developer’s skillset.
Also note that in many cases, the one reading the code will see errors that aren’t really errors, because they don’t fully understand the problem domain yet. That’s another reason to be polite and impersonal about it. :-)
Scott— While what you say is reasonable, it isn’t what Ricky said. He said, and I quote: “Don’t criticize code by your team members”. That’s ridiculous. I criticize code by my teammates all the time, and I expect them to do the same to me. That’s how we get better. Here’s the rub: you SHOULD be a jerk about it. About the code. Don’t be a jerk about the programmer. Its the difference between “this code sucks” and “you suck at programming”. The former: totally awesome and useful. The latter: gets you an M&M in the eye in our office.
Justin,
Can we let ‘ghetto’ fall out of the Ruby-world vocabulary? Call me a bleeding-heart, but I think we have better adjectives for describing “crap” than need for trivializing the socio-economically destitute of the world.
Jeff—sure. I hope you realize that I used the term, not as a reference to any pre-existing socio-economic stratifications or social constructs, but as reference to Zed’s post and stand-in for his, shall we say, forward style. It was a “meta reference”, if you will.
Yep, I follow you. I would just hate to see it become a colloquialism of our little culture.