A post written by Uncle Bob in January (I'm behind my reading list) offended me. I absolutely agree with Uncle Bob's analysis regarding the code itself, and I also prefer the refactored version, but I have a problem with insulting the programmer(s) reluctant to appreciate the change.
We write code in programming languages, and there are different levels of proficiency in a language.
As I'm currently learning a new spoken language, I'm painfully aware of this - initially I probably sounded like a caveman. The first impression you get about me is totally different depending on the language I speak - but I am the same person!
The learning curve of a language is not smooth - the steepness between consecutive levels of proficiency is different. Going from not speaking any German to speaking A1 (tourist) level was easy, getting the basic grammar required for the low intermediate (B1) level wasn't too bad, but to get my German to the level where my English is will take more effort than the sum of all my previous investments1.
Since it is my third foreign language I'm learning, I have no difficulty accepting that the level I think I speak is higher than the level I actually speak. Because of that, whenever someone rephrases my sentences in proper German 2, I start from the assumption that likely their version is better, even if I don't understand first why - and I take the effort to understand their reasoning 3. I do that despite that I was of course convinced that when I spoke, I expressed my thoughts in the best possible way.
However, I don't have much at stake - no ego to hurt, no reputation to loose, and the roles are clear: I'm the beginner, and the people around me are the more experienced ones. In a software team, the roles might not be so clear - I had told colleagues almost twice my age how they should write code after only a few weeks of working there. Bad idea. Since then, I have learned not to start improving the coding style of a team by rewriting the code they have written, and showing off how much better my version is. Rather, I wait until a situation arises when they don't mind having me demonstrate some code improvements. I demo it, and explain why I do it that way. In my experience, the second approach is more effective, though it doesn't have that instant satisfaction and relief the first provides.
As the joke goes, you need only one psychologist to change a light bulb, but the light bulb has to want the change real bad.
Driving Technical Change is hard, because it requires a mental/cultural change, and that change has to come from the inside - but can be catalyzed from the outside of course4. But just forcing practices or ways of working on unwilling recipients generates resistance (e.g.: the story of the EU technocrat appointed to recalculate the 2009 Greek budget).
I would like to see more public code reviews and public refactorings (e.g.: Andrew Parker, GeePawHill), but I would like to see less public judgement passing on people at the lower proficiency levels of programming.
1 there is a great Hanselminutes episode on learning a foreign language if interested. Beware, it may contain programming!
2 German readers might disagree, since most Germans I meet speak Frankish :)
3 Which of course, is sometimes harder for natives to properly explain than for novices to ask questions pointing out the seeming irregularities of the grammar
4 And we won't always be able to foster change in all environments (note: this does not mean the others are at fault for not changing!). The same programmer can be highly productive in one team, and be the one slowing down another team. There is nothing wrong with changing jobs after realizing we are a net loss to a given team.
bridge on 2012/02/06 11:53:39: "I had told colleagues almost twice my age how they should write code after only a few weeks of working there."
Once I did the same: I pointed out that although they wanted to write a super-fast app, writing 500-line long methods just won't work. I also pointed out that having the code polluted with //System.out.println() -s is bad, that they should use some sort of logger instead.
It was pretty obvious to me, even before I read the the Clean Code book. Still, I believe this is why I couldn't get along with the team, and this is why was fired in 2.5 months.
The other side of the coin is: sometimes you just don't want to wait until the bulb wants the change really bad.
Peter Zsoldos on 2012/02/06 20:55:06: Hi bridge,
so I'm not the only one who made that mistake :). And you are right - when there is a culture clash in a team, or we need something other than what's possible (e.g.: we mostly would like to learn from others to improve our tech skills, and not work on our soft skills to be able to teach them our - potentially limited - understanding), moving on to another places probably the best thing - even if on occassion it's a move forced on us!
- Peter Zsoldos on 2012/02/06 20:55:06: Hi bridge,
bridge on 2012/02/07 10:28:50: Sure, there must be a lot of us who made that mistake :)
On the other hand, I was asked to review their code. But, as it turns out, programmers are almost as sensitive about their code as poets are about their poems.
Peter Zsoldos on 2012/02/07 19:18:58: Giving feedback is tricky territory and requires either a built up trust or a lot of discipline from the feedback giver - even if one knows on the analytically that it's about the code, and not about the merits of the coder, a misdirected WTF (e.g.: *Who* wrote this mess?! vs. Look at *what* a mess *this* is.) can push the recipient into the defensive, preventing any positive discussion about *the code*.
Even if we can at least laugh at it (and ourselves), it still hurts - Kyle Baley gives a wonderful description of the feeling at http://codebetter.com/kylebaley/2012/01/30/qa-a-hillbilly-love-story/ :)
- Peter Zsoldos on 2012/02/07 19:18:58: Giving feedback is tricky territory and requires either a built up trust or a lot of discipline from the feedback giver - even if one knows on the analytically that it's about the code, and not about the merits of the coder, a misdirected WTF (e.g.: *Who* wrote this mess?! vs. Look at *what* a mess *this* is.) can push the recipient into the defensive, preventing any positive discussion about *the code*.
r1tch on 2012/02/07 14:19:04: I dare to agree with Uncle Bob. :) If someone is as junior to have problems understanding the refactored code, then he should know to shut up. In the story, the two colleagues claimed the original was to be easier to understand! This is obvious bullsh*t. The only thing I might (might!) allow for their help is to add comments in the isPzt() function, explaining what's happening, like:
// an item is PzT if:
// it's non-null, AND
// has parent, AND parent is nfo or discontinued
However, a company is in serious trouble is such comments are needed. Or am I just too spoilt here...? :)
Peter Zsoldos on 2012/02/07 19:36:10: Hi r1tch,
Thanks for your input and I appreciate that you did not phrased it as "I disagree with you" :) I don't think I should speculate about your degree of spoiltness (is there such a word?) :)
I *strongly* disagree with the statement >>If someone is as junior to have problems understanding the refactored code, then he should know to shut up<< though. First, I don't think years of experience is any indicator of skill level in itself. Second, effective teams are the ones when there are no (at least technical) unasked questions, and noone has to "just accept" the right way (see also: Cargo Cult).
With regards to easier to understand code - it might be easier for you or for me to understand, but when you've lived with a codebase, you have developed a certain familiarity with it, just like you can almost blindly walk to the newspaper stand on the corner. However, if someone was to alter the pathways, move some billboards around, even if it might be better use of the space, you would first be uncomfortable with it. OK, it's not a perfect analogy, but I hope it conveys my meaning.
We are also missing a lot of context here - e.g.: I likely wouldn't ask my colleagues after a refactoring to see whether it was correct or not, since the tests would catch if I made a mistake. But if there are no test cases, and this code is in a central part of the codebase, I can understand why team members would be worry of the new, untested, unfamiliar code. Also, while I'm not sure how it is on that team, but if someone would respond to me in a way that is described as "willing to put up with my change to shut me up", I would ask myself whether I might be doing something wrong.
And I wouldn't put the kind of description you put into the comments, but rather into testcases :)
- Peter Zsoldos on 2012/02/07 19:36:10: Hi r1tch,