DaedTech

Stories about Software

Jun
17

By

Code Reviews Should Be about Incremental Improvement

Category: The Life of a Programmer Tags: , | 4 Comments

Persuasion tends to happen at the micro-level, in terms of information volume and breadth. For instance, if I explain that I use “var x = 6” instead of “int x = 6” because a productivity add-in squawks if I don’t, you’re likely to be swayed if you find my case persuasive. If, instead, I explain that sometimes I use implicit typing and sometimes I don’t, and then I proceed to list dozens of heuristics, you’re most likely to tune out and, if I’m lucky, process and be swayed by one or two individual bullets that I mention. In these cases the grain of the subject matter is fine, but it works for coarser grained concepts as well: I may sway you on whether or not to write unit tests, onion versus layered architecture, or C# over Java. It isn’t the scope of the topic that matters, but rather the relative narrowness of information and message. Persuasion simply works better when the thesis is clear, and the arguments convincing and too the point.

This bit of human nature is incredibly important to understand when sitting for code reviews, both as a reviewer and reviewee. When I review code and someone starts telling me all in a rush about design patterns that they like and why these six classes communicate over a conceptual bus implementation and why there’s a private method in each class called “PreserveInvariants()” and so on, I’m overloaded with information. Are some of these things good ideas? Probably, I imagine. Are all of them good ideas? I don’t know — slow down. I’m not smart enough to leap instantly into a shared context in which the last several weeks of your design decisions and thinking are jacked into my brain, The Matrix style.

Neo

You’re not going to solve the world’s problems in a given code review. Really, you’re probably not even going to solve all of that many of the code bases’s problems either. But, you can decisively solve or address a few. Much as TDD forces you to focus on one problem at a time, I think well done code reviews follow suit. Aim for a handful of ways in which you can make the code better and upon which all parties agree, and consider that a win for the day. This limtiation to a handful of things to cover will keep the scope of the persuasion narrow enough to be effective, whether it’s the reviewee persuading the reviewer to agree with the choices or the reviewer convincing the reviewee to change things. Or, perhaps it will wind up being a bit of both.

As a reviewer, I generally read through the code, offering first blush impressions and asking questions. “Wow, this method seems pretty big.” “Why did you decide to make that method public rather than private?” There, now there’s one narrow point to address in each case, and we can make our attempts at convincing one another. You can explain by describing where this fits in within the big picture or by whatever means you feel available. This dialog lets us explore your code, style and reasoning together. But if you tell me up front, without prompting, the entire philosophy of your approach and all of your decisions, it will be overload, and you’ll fail to persuade. You’ll succeed only in confusing me. I know it’s tempting to address people’s possible objections before they can raise them (gives the feeling of control), but that’s usually just confusing.

As someone whose code is being reviewed, I let the reviewer process the code under review before the dialog begins. I try to resist the impulse to jump in and explain every facet of my design because it will fall on deaf ears until the code has really been groked, and maybe even after if I’m bombarding the reviewer with information. If I want to provide this information, a writeup is definitely the preferred way to do it since this allows the reviewer to pull the information as desired, rather than having it sprayed like a firehose.

So, code reviews ought to cover a handful of concepts only, to allow for the maximum back and forth of persuasion as necessary. All well and good, but you have a lot of code to cover, right? Well, to address this concern, have the code reviews more frequently or even do semi-regular or regular pairing sessions. If your code only gets reviewed once per quarter, there’s a natural tendency for the review to become a performance evaluation of you as a programmer. But that’s a terrible vehicle for narrow, focused persuasion. If you’re having your code reviewed three times per day, then when the reviewer notices a fourth point of discussion, he’s more likely to say, “meh, three is enough for now — we’ll get this other one next time.”

In the end, code reviews are about persuasion. And effectiveness is really what’s important when trying to persuade someone that your way of thinking makes sense. It pretty much goes without saying that the most important part of being persuasive is making a good case, but I’d argue that a close second is the narrowness of focus on the subject for persuasion. I’m unlikely to persuade you that you should copy every facet of my life, but I might just be able to persuade you pick your battles in code reviews.

Jun
11

By

The Zen of Rejection: Let Companies Go In That Other Direction

Category: The Life of a Programmer Tags: , | 8 Comments

The Courtship

There’s nothing like the beginning of a job search. It’s often born out of a time of transition or frustration, and perhaps uncertainty and worry, but no matter how it starts, there’s a rush and glow as you start to peruse the jobs that are out there. Why do you feel a rush and get a glow, no matter how unhappy you’d been feeling up to that point? Well, it’s because the world comes alive with possibilities as you look at Dice or Stackoverflow Careers or whatever. These aren’t just jobs — they’re the next chapter in your life.

And oh, how appealing they are. And it’s not just because you’re unemployed or sick of your current situation or just wanting a change of pace — it’s because they’re wonderful. They do Agile to your Waterfall and let you flex your hours instead of a rigid 9 to 5. That 401k match sure looks nice too, and they’re using the latest version of the language and the IDE. But, oh, there’s so much more. Casual dress. Free food and soda. Why, they even have a chef to cook you gourmet lunches and stationary bike-desks that you can use to exercise while you work. You can bring your dog in. They have X-Boxes and Playstations and Wiis. It’s really more like going on a cruise than going to work, and they’re beckoning to you, welcoming you, and telling you to become part of their exclusive club — nay, their family. You too can be one of the group of smiling, diverse people pictured in the “Careers” section on their website, having the absolute time of your life.

DreamJob

You submit your application. Really, how could you not? This is your chance at happiness, but now, the nerves set in. What if they don’t call you? But then they do, and the nerves only increase. What if you’re stumped in the phone screen? What if you screw up and tell them that you’re leaving because you don’t like your boss instead of what you’re supposed to say: “I’m just excited at the prospect of joining your organization?” But whew, you manage to avoid too much honesty and to secure an invitation to talk in person. Now, at this point, you put on your absolute best outfit, get in your car to drive over and make sure you get there very early, all the while telling yourself, “whatever you do, don’t be yourself — be that confident, diplomatic, smooth-talking version of yourself that will make you want to collapse from exhaustion the moment you leave.”

The interview passes in a surreal whirlwind as you meet with 8 different people, improvise on strange questions, answer with relief when you know what to say, pivot subtly when you don’t know what to say, remember not to fidget, and smile the whole time. You walk out into the parking lot, sweating under your nice clothes as you heave a huge sigh of relief, make yourself comfortable in your car, and get ready to plop down on your couch at home with a beer. But even that relief is short-lived, because now you have to wait to hear back, which makes the days seem like weeks, and weeks seem like years.

Finally, it comes: the offer. Now, it’s time for giddiness, assuming the pay, benefits and title are in line. You’ve been invited to Shangri La, and you’ve gratefully accepted. You gear up for the first day there, not knowing quite what to expect. And really, how could you? You’ve gone to great lengths to show them a completely air-brushed version of yourself in response to the unrealistic utopia they’ve shown you. So now, starting on your first day, you can see what the other looks like when it’s no longer the evening of the grand ball. Oh, but you’re already married. Better hope no one’s coach turns into a pumpkin (but here’s the catch: to some degree or another, it will, on both sides).

And that’s if everything goes really well. But what if it doesn’t? What if they decide to “go in another direction?” Oh, the rejection, disappointment, angst, and heartbreak.

Culture Shock

I’ve sort of mulled over how to make this point without sounding like a conceited jerk, but that’ll be hard, so I think I’ll just power through it and hope that most of my readership can relate. We’re programmers, and programmers are generally pretty intelligent. I’d imagine that most of you reading are no strangers to overachieving.

When I was growing up and attending school, test time was my time to shine. Whether it was getting into the advanced track math classes, taking standardized tests, trying out for clubs, applying to colleges, or pretty much anything else you can think of, those were things where I showed up and won. I got straight A’s in high school and graduated valedictorian. I won academic scholarships. I was never even cut from a high school sports team. In the sieve of primary and secondary school stratification, I was always the one that received the metaphorical job offer, and I’m sure it was the same for many of you. We grew up in a culture where a bunch of kids in a room working on a test meant you were about to get some accolades and general validation. If the real world were like high school, we’d all have had to rent storage to house all of our offer letters. Wake-ups would come later.

The first one I got, personally, was in my orientation week at college. The Carnegie Mellon CS department is pretty selective, and to prove that we were no longer in the minor leagues, so to speak, they asked all of us in our entering class of 150 people or so to raise our hands if we’d been the valedictorian of our high school. Something like a third of the hands in the room went up, and my jaw dropped. I wasn’t in Kansas anymore, and I no longer won anything just by showing up.

But if college was tough in this circumstance and some that would follow, the “real world” was downright depressing and seemed to have no interest in a fresh computer science grad (graduating at the end of 2001 as the dotcom bubble was bursting sure didn’t help). I failed job interviews before they even talked to me. I sent out resumes and they crossed the event horizon of some HR black hole. I’d get the occasional phone interview and then a “no thanks,” email if they bothered responding at all. In my life growing up, I’d barely ever heard, “I’m sorry, but we’re going in a different direction,” and now I was lucky to hear it because at least I was hearing something. The effect on my self esteem was profound. At the time, I thought my degree to be useless and I felt that some sort of confusing betrayal had occurred, rendering past academic success entirely non-predictive of continued success in this cruel new world I had entered.

Returned Mojo, Anger, and Preemptive Rejection

Eventually, I did get a job. I somehow managed to convince someone to hire me, and I was incredibly grateful and terrified. I was grateful because I could stop moonlighting and taking retail jobs to make ends meet, but terrified because if I’d learned anything from the job search it was that I was great at school but bad at life. But I made it in the real world. In fact, I more than made it. After a stumbling out of the gate, once I was employed, I consistently earned excellent performance reviews and was rapidly advanced and given greater and greater responsibility.

I started to regain some dignity and then some swagger. I was no longer “kid with no industry experience” — I was a software engineer that took business trips, met with clients and, most importantly, got things done. During this time, I also started earning my MS degree in Computer Science via night school, and by the time I hit the job market again in coming years, I was ready.

In fact, I was more than ready. I had learned my lessons from the previous job search and hit the ground running when I wanted a change, doing all of the right things. Interviews were easy to get for someone with my (now miraculously no longer useless) degree pedigree and years of experience in several programming languages, and I did well enough to receive offers. And this time, I’d figured out an important fact: any company that didn’t make me an offer clearly had a flawed interview process. Any form of rejection I haughtily interpreted as ipso facto process failure on their part. This arrogant self-righteousness, I believe, was misdirected resentment at my culture shock from the first go-round. I had done well in school, and then done well in the business world, and was getting offers from most companies with which I interviewed, so how dare those other companies and the ones from years ago make me doubt myself the way I had, very deeply, as a new grad?

As the “chip on the shoulder” mentality started to fade a bit (with me maturing and realizing this was petulant and comically egotistical), left in its place was a more antiseptic tendency to preemptively reject certain companies. I didn’t like rejection any better than at any time in my life — who does — but I didn’t view it as an insult or a mistake, either. I came closer to seeing it this way: “if I don’t apply, I’ll never be rejected, but if I apply and am rejected, I’d always have to say, if asked under oath, that I wasn’t good enough for that company.” By this time, I had become a good sport about “thanks but no thanks,” but I sought to avoid it. If, based on the job requirements or phone screen, it seemed like an interview might not go well, I told the company, “no thanks,” before they could tell me the same. Aha! The rejector has become the rejected! Take that! I would only play in games where I felt the deck had been stacked in my favor, which is good for a small king in a small kingdom, but bad if you ever want to reach and push yourself.

The Reality and Rejection Zen

I spent a lot of years going through this progression of my view on interviews. The feelings of inferiority, regaining my confidence but compensating with righteous indignation, and picking and choosing my spots to minimize rejection all played into an internal mantra of “there’s so much wrong with the interview process and it’s so reductionist, and I won’t do that to people.” If I were a character in Animal Farm, I would be Napolean, however. Like him, I wound up becoming that against which I railed. As my career wound on and I was in a position to make hiring decisions, I surprised myself by being reductionist. “Oh, that was the woman who didn’t really understand what unit tests were,” or “that was the guy who stumbled weirdly when trying to explain why he liked ASP MVC instead of ASP Webforms.” Human lives — and probably imminently capable humans — reduced to a single mistake they had made or passed over in consideration for someone who had happened to impress that day.

It was from this perspective that I realized the sad reality of the interview process. I had been viewing it wrong all along; you’re not really “rejected” from jobs and the interview process isn’t an evaluation of your intrinsic worth in the same way that something like the SAT purports to be (even at companies that make it a point to try to make their interview process exactly that). At best (and probably more in the realm of ideal), it’s an evaluation of mutual fit — something like, “we have a team with characteristics X and Y, and we’re looking for compatibility with X and Y, but also someone who brings Z.” You can be a wonderful human, full of X and Y, but not familiar with or interested in Z, and the position won’t be a good mutual fit. For instance, if I’m hiring someone to be a DBA and your interest is in client side web programming, neither of us pursuing things past the phone interview phase is not a rejection.

What I finally understood, particularly after seeing how the sausage is made and then making it when it comes to candidate selection, is that an interview is really more like you calling up a buddy and saying, “hey, wanna go see that new X-Men movie tonight?” Most likely, the answer will be no because there are a million reasons it could be no. Your buddy might be sick, he might be working late, he might have a date, he might be watching a game on TV, he might be annoyed with you, he might not like movies… the list goes on forever. Some of his reasons could be related to you and some completely based on other factors.

So it is with the interview process. A company may think you’re great and perfectly able to do the work, but have someone else in mind that’s just an absolute perfect match. Or the CEO might be forcing them to hire his nephew. Or they might have decided not to hire after all. In fact, it could be that they never intended to hire, and the whole process was a sham to humor some muckety muck (and before you doubt me, I’ve seen this happen). Again, as many reasons as stars in the night sky.

Once I wrapped my head around this, I stopped fearing or much caring about interview “rejection” beyond simple disappointment that I didn’t get a job about which I was excited or perhaps regret at the fruitless time investment. It was as if I’d called my friend about the movie and he’d said offhandedly, “meh, not tonight.” The response then becomes, “okie dokie, maybe some other time.” A little disappointment is in order if you were hoping to go together but, hey, you have other friends and you could always go on your own.

This is how I got over the feeling that interviews are some measure of your competence or worth — a feeling that I have no doubt is quite pervasive among those reading as well, even if you tell yourself or others that you’re fine with it. So next time you don’t get a job offer, imagine saying in your head, “well, maybe we’ll catch the movie work together some other time or maybe I’ll just invite someone else apply somewhere else.” It seems silly, but I invite you to try it. I imagine that you’ll find it takes the sting out of that call/email/letter/non-response to some degree, if not totally. At the very least, I hope my story might help your confidence not be shaken.

This is the end of the first part of this series, but I anticipate several more. Stay tuned for next time when I tell you why this zen state that I’ve reached depresses me in a kind of detached way and as I delve into how seriously, seriously dysfunctional I think the employee-employer pairing process is in our society (including the likely controversial assertion that the interview process might conceivably not be worth doing). By the end of the series along these lines, I’m hoping to collect my thoughts enough to end on an up note, offering ideas as to how things could be improved.

Jun
1

By

Armchair Tour Guides and Presenters

Category: The Life of a Programmer Tags: | 3 Comments

The blog has been quiet for the last 10 days or so and that’s because, as I mentioned previously, I was heading off on vacation. We had a good time; a few days in Seattle, a brief stop in Vancouver, and then a cruise along the Inner Passage of Alaska, where we stopped in a variety of cities and towns. It was a lot of fun. I got to fly over glaciers and an ice field in a small sea plane, go on a whale watching boat ride, see glaciers calve up close, and get a good look at a lot of indigenous wildlife (yes, including grizzly bears).

A lot of this was accomplished via paid tours and group outings, and, apart from enjoying myself, I had the opportunity to bemusedly observe a phenomenon that I came to think of as “armchair tour guiding.” Let me explain with examples. On my sea plane adventure, we boarded a bus to head to the docking area where the planes were tied up, and our chaperon was a girl who introduced herself as a 2-week-since high school graduate and part time employee of the sea plane company. Not wasting any time, a blowhard on the tour started grilling her about the planes — their make and model, their type of engine, and various other things that skittered out of my consciousness since I’m neither a pilot nor do I play one after reading a bit of wikipedia. The girl looked a little flummoxed, and asked the bus driver, and between them they were able to answer at least some of the guy’s questions. “As you can see, I do my research,” the man said, not bothering to hide his smugness at all. I had the distinct impression that not having all of his questions answered was the exact desired outcome. He won a contest in which only he had competed and no doubt became an even greater legend in his own mind.

On another tour on the trip (they did an excellent job, by the way, if you’re ever in a position to check them out), we took a boat out into the bay near Juneau to watch humpback whales and whatever other aquatic life we might happen to see. Our guide was very knowledgeable, and we learned a lot about the humpbacks, other marine life, and the area in general from her. That is, we learned a lot when a handful of armchair tour guides on the trip could restrain themselves from vying for center of attention. There were several, but the ringleader felt the need, at times, to interrupt the tour guide to explain that she took a whale watching tour once in Monterey Bay and learned something different. I was pretty excited to hear from vacation-lady, since clearly, my girlfriend and I had shelled out $400 for the day to hear what some woman remembered of some California vacation she took once. That was superior in every way to hearing the expert we were there to hear. /sarcasm

Why do I harp on this on a technical blog? Well, because in a strange way it reminded me of being at technical user groups or conferences. In those venues, I’ve frequently sat in a room and listened to an audience member or two completely derail the presentation with questions about minutiae or grandstanding games of “stump the presenter.” I’ve sighed and rolled my eyes through long-winded ‘questions’ whose only purpose was clearly to brag: “So, how do you think this ‘Introduction to Java’ presentation can help someone who has written 16 compilers — someone like, oh, let’s say, me?” I tweeted about this once and vulgarly called it “Q&A-hole” but I think I like the ring of “Armchair Tour Guides” (or perhaps “Armchair Presenters” or “Armchair Experts”) better.

ArmchairPresenter

Don’t be that guy. Don’t be an armchair anything. It’s tempting for all of us, including me. Gameshows and games about trivia wouldn’t exist if possession of knowledge weren’t a fierce source of human pride, and it’s perfectly natural to want to showcase what you’re proud of, but this isn’t the venue. Even if you aren’t deterred by how rude it is to hijack a session, then be deterred by how unimpressed those around you actually are that you googled airplanes before a plane tour or that you once went on a tour somewhere like this tour or that you’ve played with the technology about which someone is presenting. Seriously — no one cares. Ask yourself who those in attendance have paid or come to see, and if the answer isn’t “you” then you’re going to have a really, really hard time stealing the show. People don’t walk out of those types of presentations thinking, “wow, I’m glad that one guy showed up and hijacked the presentation and man am I impressed with how much he knows!” They walk out thinking, “if that idiot is in another talk I plan to attend, I’m leaving.”

By all means, engage and ask questions of presenters, but, as an exercise, ask yourself whether others would find the answers to those questions interesting or helpful. If the answer is “probably not,” then wait until the presentation is over and engage the presenter one on one (or email him or her later or something). And, if you’re going to interrupt to share your own experiences, a better rule of thumb is, “don’t because no one cares.” Now, is this universally true? No. I’m sure there are examples you can think of where some witty audience member interjected a quip or corrected a presenter and it was actually valuable and helpful, but that is oh-so-rarely the case, compared to the number of times people do it, assuming it will be the case. I’m not offering hard and fast rules of existence, but rather guidelines for behavior that will make you far less likely to inspire annoyance and loathing in your fellow humans. It’s amazing how far common courtesy goes, sometimes.

May
23

By

Information Exchange Realpolitik: Pushers and Pullers

Category: The Life of a Programmer Tags: |

This is going to be another one of those posts where, in an attempt to explain myself, I may wind up putting my own bizarre pathologies on display for the world to see, but c’est la vie. I think that I can start most easily by saying that it drives me insane when people interrupt me to finish my sentences, though I’ve mostly mastered the art of completely suppressing any reaction for the sake of politeness. This is unfortunate and somewhat unfair of me since I tend to be methodical and precise with my words when speaking, often favoring a few beats of silence over verbal pauses or simply using a word that isn’t exactly the one I want. (As an aside, this does tend to come in handy because I write almost exactly the way I talk and vice versa, making scripted speeches sound relatively natural). In other words, I sort of invite people to finish my thoughts and then get annoyed when they do. People who finish the sentences of others I’ll call “information pushers” or just “pushers.”

I wanted to expand on my recent post about the great TDD debate to go into more detail about the real, underlying reason that people shouting their opinions at one another bothers we and one that goes well beyond simple matters of social decorum. It isn’t that I’m a shrinking violet, wishing that everyone would just stop with all of the shouting and be friends. It’s that I have an aversion to people spewing unsolicited opinions — I have an aversion to being around people who are serial pushers. It isn’t that I dislike the people, per se, it’s just that they’re hard for me to take in anything other than small doses. This is probably because I’m generally not a pusher and I’m only slightly a “puller.”

That last statement probably seems odd because any lover of symmetry would assume that, since I’m not a pusher, I must be a (as yet undefined) “puller.” But these things aren’t mutually exclusive. One can be neither or one can be both. I’ll define a “puller” as someone who tends to solicit information and opinions. You’ve probably worked with intense pullers in the past — people who can’t seem to go 10 minutes without asking you for help with something. Some people are pushers and pullers (I think that a lot of extroverts are both), blasting out opinions whether or not others are interested, but also genuinely curious about the opinions of others. A lot of gossipy people are both pushers and pullers.

Others are neither. Reclusive people, for instance, don’t care about your opinion and aren’t interested in telling you about theirs. This comes closer to describing me. I rarely offer opinions or advice unless people specifically ask me for them. That probably seems strange coming from a blogger who has been pumping out opinions for years, but if you think about it, by reading the blog, you’re sort of asking for my opinion. It isn’t as though I’m sending you an email telling you what I think about Java’s new implementation of generics or dropping by your cubicle with a cup of coffee and a “Hey… whaaaaat’s happening. Good.”

I have plenty to say if you ask, but I usually leave you be if you don’t. On the other side of the coin, I am a puller, but intermittently. I have a strong preference for trying to figure things out and master them on my own, and I tend to solicit help on problems only when I feel I’m in a blind alley. I’m more of a puller when it comes to passive assimilation of information — I read a lot of blogs, books, etc. But when I’m trying to solve a problem, I really only want help when I ask for it.

And therein lies the crux of the matter from my perspective. What happens when a reluctant/infrequent puller is working on a project with a huge pusher? Well, from the perspective of a slight-to-non-puller, what happens is that awfulness ensues. Someone I work with has a great expression that she uses that is appropos to this situation: “I asked you what time it was and you’re telling me how to build a clock.” This is a situation where a slight-to-non-puller is blocked, in need of some information from a pusher, and winces at the prospect.

Non Puller: Hey, can you give me the login credentials so that I can hit the web service from my code? I’m trying to —

Pusher: Oh, well, what are you trying to do? You probably don’t need a web service at all and could just get by with the old protocol we were using before. Let me take a look at your code. Yep, I don’t think you need any of what you were doing, and while we’re at it, I’ll just take the liberty of explaining to you that Java is a managed language, which means…

Non Puller (Mutters): Kill me, please.

Pusher: What was that? Anyway, like I was saying, programming can be summarized thusly…

Is this familiar to anyone out there? You need to ask Bill for just one tiny piece of information, and you know that as soon as you do, he’ll dash off to the nearest phone booth, don his Captain Obvious outfit, and return with a torrent of unsolicited information. So what do you do? You avoid Bill because he’s insufferable. Perhaps you make subtly snarky comments about him at lunch with the group. But what you probably don’t do is lose it and tell Bill that every word out of his mouth takes a fraction of a point off of your IQ. Even if you did, he’d probably interrupt you half way through your rant, impervious to your ire, to explain that IQ stands for Intelligence Quotient.

CaptainObvious

Don’t be Bill. Don’t be a pusher. It’s tempting at times for anyone. I mean, humans like expressing opinions. I’m no different, but I’m either wired or else I’ve conditioned myself to tend to do it only when solicited. When I feel like offering unsolicited opinions, I come here and vent them, but, as I’ve explained before, they’re not really being forced on anyone.

It’s not so much that I’m saying, “be like me because I’m awesome,” but what I am saying is that being a non-pusher has definitely helped me extensively in my career. I listen to people’s problems rather than interrupting them with solutions they’re not seeking. I let people finish their thoughts. I listen. And I offer opinions… when asked (for the most part, anyway — probably everyone is at least slightly a pusher, at times).

And, honestly, it’s opened doors for me. People tend to find me trustworthy and pleasant. Often times, near-strangers start telling me about rather personal problems. I tend to do fairly well in job interviews. A lot of it’s politeness and common courtesy, in my mind, but it all stems from avoiding tendencies to be a pusher.

So, here’s my take away from the situation. From an office realpolitik perspective, you should avoid being an extreme puller, since that’s annoying and it makes you seem incompetent. You have to be able to solve some problems on your own, but slight to modest pulling is the way of the world, since we don’t know everything. Ask judiciously and, when you do, demonstrate a good faith effort at solving the problem on your own.

But when it comes to pushing, I have no “go the middle route and avoid either extreme” advice. I can unequivocally say that I think pushing is a bad use of political capital in that it’s either all bad, or it’s a slight short term benefit for a long term detriment (i.e. you gain short term status as an aggressive expert, but eventually are thought of as a blowhard like Master Beginner). Spewing forth unsolicited opinions is like the influenza virus — the less you have of it, the better. Period.

So save that energy and do something useful with it. Heck, channel it into a blog. Or focus on being really good at what you do, which will have the nice effect of people actually soliciting your opinions, at which time you’re free to hold court. Respect is earned through actions and proof — not attrition.

May
20

By

How to Use a Code Review to Execute Someone’s Soul

Category: The Life of a Programmer Tags: | 31 Comments

I was having an interesting email discussion with someone not too long ago, and he made an offhand comment about code reviews that I noticed, identified with, and thought was probably a common experience. In essence, he said that he was yet to see them done well and be anything other than depressing experiences. For me, that’s been the case with most, but not all, code review environments. But before throwing the baby out with the bathwater, I’m hoping to write a how to guide for creating dirty bathwater in the hopes that you’ll read it and then not create dirty bathwater. By which, I mean, not give bad code reviews.

I have an odd personality characteristic that compels me to watch or listen to things over and over again if I find them compelling. That is, you might say, “hey, this song’s pretty cool,” and play it for me. If I really take to it, I pity you, because there’s a chance that I’ll play it over and over for like 45 minutes or so. I always feel sort of like a neurotic weirdo, so I try to play it cool and not do it in front of others too much, but, whatever. I’m getting less and less inclined to conceal my weirdness as I get older. I mention this as a segue to explaining the post title.

When I was younger, I watched the Al Pacino movie, “Scent of a Woman,” and I came to be very fond of the Al Pacino character, Frank. He said a lot of very memorable things in that movie, in kind of a gruff, misanthropic, wolfish, but nice-guy-when-you-get-to-know him way. I won’t go through all the quotes here, but I probably could. The reason I could is that I watch things enough times to commit huge swaths of them to memory, and this was the case with this movie. Indeed, this is the reason for the neurotic replay — I don’t just want to experience things; I want to command them and be able to recall and recite them at will later. Anyway, near the end of the movie, Frank gives this extended, loud, angry soliloquy defending his reluctant protege, who is on the precipice of expulsion from school. During the course of this, when talking about the effect that expelling him would have, he says, “you think you’re merely sending this splendid foot-soldier back home to Oregon with his tail between his legs, but I say you are executing his soul!”

What he’s referring to is that the boarding school is on the verge of expelling the protege for taking what he feels to be a principled stand and refusing to “rat out” a classmate in exchange for favorable treatment. The Frank Slade character is saying, in essence, “by punishing this kid for ethical behavior, you’re jading him and altering the course of his life and interactions with others.” And I think that this sentiment has a parallel in our world of programming: I have seen code reviews, things that should be a collaborative, learning experience, subvert group dynamics and result in negativity and hostility being paid forward. So, if you’re interested in a guide on how to do just that and how to turn optimistic, bright eyed developers into angry, cynical people, look no further, because I’m going to tell you.

Nitpick and bikeshed

I think most of us would agree that there’s nothing like spending an hour or two arguing about whether to use implicit typing or not or whether it’s better to trap a null parameter and throw ArgumentNullException or to just let the NullReferenceException happen organically. So, why not make all of your code reviews like that? You can forget about big picture concerns or pragmatic but philosophical discussions such as code readability and ease of maintenance and opt instead to have heated arguments about the little things. Anyone who had previously viewed code reviews as opportunities for learning and collaboration will quickly come to realize that they’re the place for heated disagreements about subjective opinions on details.

Be condescending and derisive

Pointing out a possible logic error is something that would go on in a healthy code review, but why stop there? If pointing out the error drives the point home, adding an “I can’t believe anyone could miss that” will really light a fire under them not to do it again. Since your code reviews are one-way activities, there’s no chance of someone on the other side of the table noticing your mistakes later, so you might as well let ‘er rip with the insults and really dig into their self confidence. They’ll learn to look forward to the day when they can follow in your footsteps and humiliate junior developers from right where you’re sitting.

ExpertBeginner

Make them interminable

If a little code review is good, a 10 hour marathon has to be great. Make sure you go over every line of code, every config file change, every markup tag, and every automated refactoring. Bonus points if you get things wrong in the history and review things that other developers checked in, taking the one in the review to task for code that he didn’t even write. Punish developers for getting their work finished by making them justify every character they’ve added to the code base as if it were a PhD Thesis defense.

Make the code reviews gate-keeper reviews that have to be “passed”

While you’re on the professor-student theme, you might as well go ahead and implement a policy that code has to “pass review.” That way, it’s not a team of professionals reviewing one another’s work and offering critiques, suggestions and insights, but rather a terrified junior developer trying to figure out if she’s good enough to check in code this release. Why not make every sprint/iteration/release just like taking the SATs and waiting for college admissions decisions for all involved? There’s nothing like wondering existentially if you’re good enough at life to have a future to keep the creative juices flowing freely.

Pass your opinions off as facts

Is it your opinion that static methods are better than instance methods? No, of course not. It’s just a fact. At least, that’s how you should approach code reviews. Think of all of your personal tastes in design patterns, coding approaches, style, etc, and then remove qualifying phrases like “I think that,” “I prefer,” or “In my opinion.” So, “I like pre-pending field names with underscores” becomes “you should pre-pend your field names with underscores” and “I find out parameters confusing” becomes “out parameters are bad.” And whatever you do, never back anything up with evidence or citations. When you tell someone that their methods are too long and they ask “what makes them too long,” don’t cite any studies about the correlation between method length and increased defect counts, just see the point about condescension and derision and say something like, “the fact that I’m the senior programmer here and yours are a lot longer than mine.”

Cover things for which checks could be easily automated

Why spend the day on productive tasks when you could sit in a cramped meeting room and pore exhaustively through the code looking to see if anyone committed the grave sin of using camelCase instead of PascalCase on a method. While you’re at it, count the lines in each method to see if they’re too long. Maybe even compute the cyclomatic complexity by hand and then have arguments over whether this method’s is 4 or 5, if you count the default of the case statement. Sure, there are tools that can do any of those things in seconds, but then you lose out on the human touch whereby junior developers are publicly rebuked for their mistakes instead of learning about them quietly and quickly from an automated tool.

Focus only on the negative

It’s amazing how just a little bit of positive feedback can brighten the whole experience, and there should be plenty from which to choose, since examining others’ code always presents an opportunity to see new ways of doing things. So, because of that, you have to be careful to keep it all negative, all the time. Use a whiteboard or a spreadsheet to list people’s mistakes, and the more, the better. Everyone should walk out of code reviews pretty convinced that they chose the wrong line of work and that they can’t hack it. Lights at the end of the tunnel are for cowards.

The Takeaway

I’m hoping it’s pretty obvious that this has been a tongue-in-cheek post, but there’s a serious takeaway here. It’s easy to allow code reviews to become negative, depressing and even political affairs. I’ve heard an awful lot of developers talking about hating them, and I think it’s for a lot of the reasons mentioned here — they’re needlessly adversarial and draconian, and people learn to dread them. I also think that there’s kind of a human nature tendency to “pick on the new kid” and make these things tiered affairs where the council of elders passes judgment and doles out tribal knowledge.

To keep code reviews healthy and beneficial, I believe that a concerted effort is required. I might make a post on this as a separate subject in the future, but some general ideas all revolve around staying positive and respectful. If you see things that could be errors, you don’t need to tell people they’re wrong, usually a “what do you think would happen if I passed null into this method” would suffice because the person will probably say, “oh, I didn’t think of that — I’ll fix it when we’re done here.” Allowing them to solve the problem and propose the improvement is empowering and so, so much better than giving them orders to fix their deficient code.

Look at code reviews not as exams or activities to prove the worth of the code, as-written, but as opportunities for groups of people to work toward better solutions. Instill a collective sense of code ownership instead of different people slamming their visions together and watching sparks fly. Reduce the friction in code reviews by complementing the activity with pair or mob programming. That way, more people are defending the work together than the group picking on a single person, and you also get the benefit of not spending days or weeks building up a defensive attachment to your code.

I have seen studies suggesting that the most effective way to reduce defect counts is not to try really hard or run static analysis tools or even (hold your noses, TDD fans) unit tests. The best way, bang-for-buck, to reduce defect count is through pair inspection. It’s far too valuable a tool simply to toss. We just have to be decent about it and treat one another like human beings.

By the way, if you liked this post and you're new here, check out this page as a good place to start for more content that you might enjoy.
« Older Posts
Newer Posts »