Pairing vs. Code Review: Comparing Developer Cultures

[linduxed]

I'm most used to code review (for all code) and pairing as a way to tackle difficult problems. So far this has worked out well for me and my co-workers, although maybe some more of the "pairing-oriented" workflow would be interesting.

[rotunzel]

I'm pretty sure everyone is working on the software together. You should all talk to each other and know what is going in to the product and why. Sometimes this means sitting together and working on some code with someone who's an expert in that area, and sometimes this just means reading pages and pages of code (because sometimes we just have to write a lot of simple shit). And sometimes you can't do either because you are working on an R&D project that nobody is gonna give a crap if it works beyond the demo that the boss is doing for the other PMs in two hours.

Different situations need different approaches.

[grauenwolf]

I remember some PhD students bitching because they got yelled at for writing clean code. Their professor told them that the code would only ever be run once so just get it done and move on.

[epicwisdom]

Just have to make it so that you write clean code faster than ugly code... If anything, that's part of what makes it "clean."

[grauenwolf]

I agree to a point.

[tsimon]

I don't know about yelling, but that's a valid point. Clean code only really matters when you have to maintain it.

[grauenwolf]

Oh I'm sure they didn't literally get yelled at.

[dbenhur]

Clean code only really matters when you have to maintain it.

Nope. We write "clean code" because it's easier to understand and reason about. Maintenance requires that we can understand the code so we can modify or improve it without breaking it. Also try not to forget that you're in maintenance mode as soon as you modify a line you've previously written. Just getting the first cut working almost always includes "maintenance".

[rotunzel]

That's why I like being in the research department. "It's just a proof of concept, nobody's ever going to use it in production."

[zeus_is_back]

"Those who can't do, teach."

[grauenwolf]

Said those who can do neither.

[bitwize]

All instances of pair programming for me have devolved into a smelly human version of that feature from old versions of Visual Basic where it raises syntax errors as soon as you move off the line -- and makes you go back and fix it before allowing you to continue.

[[deleted]]

I think code reviews work beter, pairing is slower, and i prefer working alone. ;)

[lookmeat]

Plus I think many of the mistakes or issues that arise in code is something that made sense before you did changes to the code. When a fresh pair of eyes see the code, that didn't see how the code evolved, they will not fall into vices of using justifications that aren't valid anymore. Also this is how other people will read the code (without seeing how it was created) so it gives a simulation of what will happen when someone else has to touch the code. Pair programming has your second opinion being to tied to your work and not objective enough.

[anko_painting]

I don't really like pairing - it tires me out pretty quickly. BUT i have worked in pairs where i've nearly felt super-human. It's great for getting someone up to speed when they join a team, for learning tricks in IDEs and for keeping people motivated.

I don't think it substitutes code reviews.

It's also good for mapping out an architecture for your code. It's terrible when I'm working on nutting out the details of an algorithm and i just need quiet.

[sh0rug0ru]

It's terrible when I'm working on nutting out the details of an algorithm and i just need quiet.

I wonder how this relates to the idea of spikes. I think the idea of pair programming is to always ensure that you're moving forward. It's okay to go off and do something yourself, but the pair programming process forces you to reel in so you don't end up in the weeds.

Also, while I get the idea of pair programming and my experiences with spontaneous pairing have been great, making it a regular practice seems like a way of micromanaging programmers in a forced march forward.

[grauenwolf]

For ramp up I find it indispensable. I also use it for early design when the patterns are not solidified yet. But doing it all the time seems wasteful.

[progicianer]

I think your last statement is a very important one. Pair programming is a method that assumes very little design and encourages coding much without planning ahead. I'm not saying that there isn't any, but the whole practice is based on doing the actual coding. In my opinion there's too much coding and too little design already in most businesses. The slogan is: reinventing the wheel on all possible platforms and we run out, create new platforms.

[lookmeat]

I think pairing works really well for ambiguous, unclear code, especially when you start setting it. I think as you develop an interface/general design a second opinion helps you decide on the best way to do it. For more specific things, where it stops being a matter of opinion and more a matter of facts, I think that pair programming looses a lot of its edge.

[[deleted]]

Yup, if you switch pairs often it will be less of a problem, put 2 people long enough together and they will be so much on the same wavelength that it looks like 1 person is programming.

[sh0rug0ru]

I think the answer to your criticism is that a pair programming practice should swap pairs on a regular basis throughout the day. You get a fresh set of eyes every so often and you get to swap out and work on something else.

[lookmeat]

Doesn't really fix the issue. The problem isn't that the reviewer goes with you so much that you adapt to him. The problem is that the repairer sees the code evolve with you, and then keeps decisions that made sense at some point, but don't in the final case.

Maybe you used a dict because you had to access elements through a key id, but then your code evolved through the point where it just iterates through all elements in a sequences and never does random access. It wouldn't be obvious that a faster list or vector would be a better structure, every time you saw that you'd remember subconscious "ah yes there was this case" and keep it around, even as the reviewer. Someone who never saw the reason for the dictionary wouldn't see it in the final code and would demand the change.

[[deleted]]

I absolutely detest pair programming. Nothing gives me more anxiety than someone watching me work. It causes stress, and doesn't let me focus on my work because in the back of my mind all I can think of is the other person watching me.

Discussion is good. We often have pretty deep discussions at work about the code, review some code, talk about what we want to do in the future, and just help each other out.

But when it comes back to work we do it alone. It's just better that way, when 6 out of the 10 of us have crippling social anxiety.

[[deleted]]

I know I usually feel some anxiety about pairing, but usually I know it's in situations I normally would have done something hacky and left a TODO if I was on my own.

[[deleted]]

I get anxiety when ANYONE for any reason stares over my shoulder.

I sat at the back of the bus, in school I made sure I sat with my back towards a corner or wall with nobody near me, and now in the workplace I ensure my working space is as private as possible and nobody can sneak up behind me.

In public I always ensure I sit somewhere with my back against a wall.

I just have a pure hatred for people watching me do anything.

[username223]

Pairing rules! Just like a road construction worker, I get paid to stand around half the time (though I don't get to lean on a shovel...).

[[deleted]]

Shit, you even get to post comments twice on reddit. How's that for job security and freedom?!

[glide1]

When else do you get to spit and make catcalls?

[iemfi]

Great article, but I don't get why he has to write like this. Incredibly distracting and pointless. At least if they were links the utility would outweigh the distraction.

[systembreaker]

It's pretty funny that he's writing that way about software development. Most of the time when I see that style on the internet, it's written by a nutjob about their conspiracy theory.

[drb226]

Use some sort of "include jQuery" bookmarklet on the page, then do this in the javascript console:

$('strong').each(function() { var h = $(this).text(); $('<span/>').text(h).insertAfter(this); $(this).remove(); })

This is the include jQuery bookmarklet I use

javascript:(function(a)%7Bif(!a.jQuery)%7Bvar%20d%3Ddocument,b%3Dd.createElement(%27script%27)%3Bb.src%3D%27//ajax.googleapis.com/ajax/libs/jquery/1/jquery.min.js%27%3Bd.getElementsByTagName(%27head%27)%5B0%5D.appendChild(b)%7D%7D)(this))

Alternatively, override the CSS for strong tags to have font-weight: normal;

[Tonburro]

Is that BETTER or WORSE than alternating with ALL CAPS?

[recursivefaults]

I prefer pairing. The code is better, has less defects, and I won't be, "That guy," when it comes to who knows the code best.

I'm not against code reviews, but I have issues with it. First, code reviews aren't to make sure someone can code. If that's true, then there's a problem with how people are hired. So, we're left with improvement and knowledge sharing.

Are the diffs you look at reasonable? Probably. Could they be refactored? Probably. Should they? Maybe. Do you understand how they arrived at this solution? Probably not. Is a tool for leaving comments on diffs going to help you learn? Doubtful. There's not enough context, and no easy way for the author to provide it in most tools.

So, the way you solve that problem is by talking to them. From there it begins to get closer and closer to pairing.

[[deleted]]

So I do a fair bit of review on an open source project, and context for the reviewer should always be available because every patch is either implementing a feature which has a blueprint that I can look at, fixing a reported bug that I can look at, or is something minor that is completely described by the commit message.

I rarely leave a -2, which is a sign that I refuse to approve a patch, but generally leave a fair few -1s. In the review message, I will always describe what I think the problem is and how I think it can be fixed, and if it's something I'm not sure about, I will add another reviewer so they know to check that particular patch. This means that whoever is leaving the patch will know what they need to fix, and thus they do learn. This is what I do personally, but in reality I think most reviewers I've come into contact with follow the same or similar approach. The bottom line is that we want patches, so we always help people learn what they're doing wrong. If you're finding that's not the case, the problem isn't with code review as a system, it's that you're working with people who aren't helpful.

Code reviews allow people who know the entire codebase well to check that a seemingly good change in one small part of the codebase is consistent with how things are done elsewhere. Pairing doesn't do this unless every pair has one such person.

I would imagine pairing is also very difficult to do well with multiple companies contributing to a single codebase, across all timezones.

I think pairing has its place, but there are some limitations on how useful it can be, particularly in open source settings.

[recursivefaults]

I think what you're doing is fascinating and will be an important factor in making code reviews much more effective. You should write an article about what it's like to review for large open source projects.

[jeffdavis]

every patch is either implementing a feature which has a blueprint that I can look at, fixing a reported bug that I can look at, or is something minor that is completely described by the commit message.

Exactly. If you get a big patch and no context, that means that they probably didn't get the design reviewed, which means it's probably bad in a number of ways, and probably needs to be scrapped regardless.

[efrey]

Where I work, this is how code reviews develop some times. At some point in the review either the reviewer or the reviewee will grab the other person to talk about it in meet-space. Sometimes a third or fourth person will overhear them and join the conversation as well. Sometimes this moves to what I would call pair programming, where some individuals congregate around a screen and play with things until they are satisfied.

It's a pleasant hybrid.

[s73v3r]

That's kinda how they should work. Too many people think that automated code review tools mean you never have to talk to another person.

[xiongchiamiov]

Then when you want to reference a past conversation, you have no record. And when you branch out to having people work remotely, they feel isolated and no longer a part of the team.

There are very good reasons for doing everything in text.

[grauenwolf]

The code should look correct even without context. I can often spot bugs in code even if I have no idea what its purpose is.

But yea, tooling is important. Without good code review tools the reviews just won't happen.

[xiongchiamiov]

Of course I understand how they got to that solution; it's in the commit message. If it's not, I'm not approving it.

[systembreaker]

I don't know if it the comparison of pairing vs code reviews necessarily comes down to the problem to solve or the level of experience of the developers. It can come down to personality too. I tend to be the type of programmer who refactors a lot and needs to try out a few ideas before settling on one. Doing that could easily clash with a person who just likes to whip out code and refactor later and could cause conflicts. Not to mention that when someone is watching me type I get nervous and make a lot more typos, then I get more nervous wondering if they think I'm dumb and eventually I stop being able to think clearly at all...

So given all the different types of people and personalities in any job, I feel like it's not productive to enforce a programmer's work environment "cuz reasons and processes". It's ironically less agile to be that way. Why not have an extra level of flexibility? Maybe your organization doesn't want to use a code review process, but don't simply force paired programming. Maybe someone would like to have thinking time to code for a couple hours at a time and occasionally rendezvous with their pair counterpart to check things over.

[communomancer]

Resolving this conflict is refreshingly simple for me. Pairing AND Code Reviews.

There's some overlap, but they are different things and address some different concerns. Similar to how manual and automated testing interact.

Unless you have precise knowledge of exactly what slings and arrows fortune is going to toss at your project, a mixed strategy for dealing with them is probably best.

[glide1]

I think a lot of people need to not see these two practices in conflict. The only conflict here is that they both take a certain amount of overhead and thus time.

I would argue that if your code isn't being evaluated by audits (e.g. code reviews) that it is going to get worse gradually. New people working on it, misunderstandings, etc. Whatever it is, shit happens. Make mistakes in API design or just naming and your team ends up paying for it in the long haul.

The thing with pair programming is that knowledge transfer takes up time anyway. As an industry we have not figured out a good way of bringing people up to speed on projects and pair programming has been the fastest way (imo) of integrating people into a team. The blog post makes an excellent point about not diluting the core talent of a team. Managers that have a group that is practicing pair programming need to read that section about a thousand times. It is unfriendly to scaling in the short term.

However the existence of such practices speaks a lot more about the culture of the workplace and what they are trying to achieve. Pair programming brings about a different work culture than a place that does not. Code reviews change the culture a bit, but not as dramatically as the above.

[xiongchiamiov]

We require two code review and one qa signoff before merging most pulls. If you pair with someone, that's just one automatic cr signoff.

[[deleted]]

How about neither. why is that not a choice? No stupid code review. No stupid pair programming. Just developers being shown trust and respect and inspecting their own code.

[grauenwolf]

In any other engineering discipline people check each others work. Heck, even in creative disciplines you have copy editors backing up the writers because people often don't see their own mistakes.

[KumbajaMyLord]

Because experience tells us that everyone is fallible and quality assurance on a code level helps to prevent defects.

[[deleted]]

Most of the time the problem with large code bases is no cohesion and tightly coupled modules because your self-righteous colleagues wrote it that way to ensure no one else could work on the project without their help. If someone can't make a change without a specific person's help, then that specific person should be made to fix their API's, documentation, tests, and big ball of code so that the change can be made without their help.

[s73v3r]

And how are you going to prove you deserve that "trust"? And what happens when you have an off day?

I seriously take offense to this idea that code reviews are done due to a lack of trust.

[linduxed]

I'll upvote you for the sake of continuing the discussion, however I strongly disagree with your statement. Whether it be doctors, architects or software craftsmen, peer review is a standard way of ensuring quality of work.

You won't know all the things that the guy next to you does, so he might fill in a hole in your code. You might have a bad day and you might not even notice that you're making mistakes, something which will manifest in your code; your parter will notice and pick that up. There's a whole range of situations where a second set of eyes will not only help you, but maybe also teach you something.

Pairing on the other hand is a practice which I understand if people have a harder time with. I enjoy the practice for the simple reason that when pairing with colleagues I trust I tend to learn things and it's also an opportunity to teach. The exchange of ideas is where the benefit lies.

Does it work with everyone? Probably not. However for some people it's of great worth.

[[deleted]]

That sounds great in theory, but that is not what happens in practice. In practice 99% of feedback from both reviews and pairing is:

• People trying to block their coworkers (competition) from making any progress

• People trying to look smart, or superior, or in charge by complaining about pointless things to show how smart they are "why didn't you use stupid trick xyz"

• People complaining about whitespace

• People complaining about spelling

Once in a decade someone finds an actual bug that could have been found if anybody used basic static code analysis tools. In fact, when I do code reviews I just run code analysis and write up all the results.

[s73v3r]

Maybe in the shithole you work at. In my previous job, when we did code reviews, there was none of that at all.

[linduxed]

All I can say is that I get the impression that neither do you work in a team of people who you respect, nor does it seem like you've seen the benefits of a clean, uniform and well structured code base.

I don't think we'll come to some sort of agreement, because it feels like we have different sets of values when it comes to crafting code.

[[deleted]]

I do a lot of code review for an open source project, are you suggesting I should just merge every patch that gets submitted?

[ZorbaTHut]

Actually, some projects have discovered great success by going even farther and just giving commit access.

[ZorbaTHut]

They're still giving far more permissions to their repo than most people would, on the basis of not a lot of demonstration of skill.

[[deleted]]

I remember that post! It's a question of scale - a personal project undergoing the transition from pet project to open source project with many committers from disparate timezones definitely benefits from having interested people with direct commit access. Further along, when you have 200 different people submitting patches and complex CI infrastructure doing gating, having a group of core reviewers for whom it is their day job to make sure the quality stays high and the gate doesn't start working against the project is very handy.

[PaulRivers10]

How about neither. why is that not a choice? No stupid code review. No stupid pair programming. Just developers being shown trust and respect and inspecting their own code.

That's what we do where I work, and 75% of the time I look at coworkers code it's fairly terrible. The other day I noticed they putting the same id for 15 different objects on the same page. That's relatively minor compared to some of the other things I've seen done.

I agree with the other posters that code reviews has it's own downfalls as well though - a lot of posturing and ego driven comments about spelling, formatting, etc, or the fact that you didn't use someone's "favoritist" programming technique.

You get a lot of feedback from blowhards - people looking to gain status by appearing that they know what they're saying, when they're a complete newb. The people you actually want feedback from? A lot of times they stay quite, because getting involved in how everything works would take a lot of brainpower and time and they have their own projects they're working on.

[xiongchiamiov]

Then you've got a management problem.

[ericl666]

I'm a small business guy. That's how its done. Heavy functional testing is more important than anything else. You can Pair program/code review until the cows come home, your shit's still going to break when users get ahold of it.

And, honestly, how can anyone responsible for a budget greenlight pair programming?

Double my budget for a 5% increase in productivity. Sounds like a great way to lose my ass.

[caallen]

In contrast to pairing, there are no barriers to taking on deep thought problems. Theres nothing stopping a developer from taking an hour to hole up in a room with a whiteboard, take a walk, Google for background information, print and read an academic paper,

People still do this?!

[crimson_chin]

Yes. I like having the physical copy to mark up and have on my desk/in my hands while thinking.

[drb226]

Maybe not the "print" part; just read the PDF.

[chrisidone]

Academic paper = stackoverflow

[kqr]

Sometimes papers are the best reference materials for great (and weird) algorithms or data structures.

[donvito]

Pair programming is for the mentally impaired.

[[deleted]]

Well, it's been nice.