Slashdot Mirror
Code Reviews vs. Pair Programming (mavenhive.in)
An anonymous reader writes: I've spent nine years working in teams which religiously follow pair programming. I'm used to working that way and appreciate the benefits it brings. We didn't have the luxury of pair programming all the time in my last project. This required us to do code reviews to ensure the quality of the code we delivered. This post is an attempt to consolidate the upsides and downsides of doing code reviews instead of pair programming in my three months of experience.
You have gone through nine years of pair programming and haven't shot yourself? The idea of pair programming itself is insane. I didn't know anyone actually did it.
Is that the new name for doubling up in the cubes to squeeze the most performance by square feet?
...and it is rather simple.
Design > Discuss > Refine > Implement > Run code analysis
This can be done in any workflow and in pairs. You don't gobble up the time of two team members but you still get to discuss the most important part of the implementation. You can even do a light-weight review afterwards with a check list of practices to focus on.
Watching someone else code is the most maddening thing. They always seem to take the long way of doing something; use the mouse and doing eight clicks where a keyboard shortcut would do, etc. I do my best to not watch people code when I'm trying to help them. I would have killed someone years ago if I did that full time.
-SaNo
This required us to do code reviews to ensure the quality of the code we delivered.
Ensure quality - that's adorable.
It must have been something you assimilated. . . .
A proper peer review process is far superior. Review your plan with your teammates. If you're working with components that others work on, make sure they are part of the plan review. Once all questions are answered, put your headphones on and go forth and code. Rely on unit tests to catch the obvious problems, rely on integration tests to catch less than obvious problems, rely on QA to catch what your integration tests miss. Use linting, static code analysis, and other tools like Sonarqube to identify potential problems within your code that may not manifest themselves under day to day usage. Voila...
Is it perfect? Of course not. Will your software be 100% bug free? Of course not. It also doesn't solve problems related to lack of intelligence or experience amongst your teammates, and it doesn't solve problems related to lack of foresight from management who impose impossible deadlines or who close deals with customers that include features which don't exist. But the team will still be more productive than if they had to share computers and work in pairs. Programmers need focus and pair programming will ruin any focus you could have.
I'm god, but it's a bit of a drag really...
I don't know if I could handle that. Does the dude's beard scratch your balls?
*What a nightmare.*
I'm glad I don't work in your hipster Gulag. The idea that someone is going to be sitting next to me, breathing through their mouth while watching every correction and backspace I make is positively frightening.
You want to review my code when I check it in, I have no problem with that, but stay the fuck out of my personal space.
>> I've spent nine years working in teams which religiously follow pair programming
I've been in software development for 18 years and pair programming has always sounded like a myth to me - I've never actually met anyone doing it for anything other than learning a new technology.
>> required us to do code reviews to ensure the quality of the code we delivered
Hell, at half the places I've worked code reviews were considered an unnecessary luxury. These days I'm just happy to hold the line at TDD - the tests pass in a reasonable amount of time, everything gets checked in and you move on to the next item on the list.
Short answer: India. Longer answer: Bangalore, India.
FIGHT!
Any designs and code developed in my projects is going to get peer reviewed, whether pair programmed or not.
> A proper peer review process is far superior.
No, a review process is useless. Proof: stagefright, where security holes and multiple (4 I think) incorrect attempts to fix them went through the code review process.
You need actual code reviews for it to work, not just the process where someone has a 20 minute sleep while having the review page open and then presses "approved". The latter is what actually happens in most companies.
How do you spot companies where code review doesn't work?
In my experience, just ask if they have commit emails with actual diffs of the changes. Because that is the only high-throughput way to get a basic level of code-review (by itself it is not a very reliable method though). If they only use tools they most like will end up with the code reviews not actually being done (beyond clicking the button or whatever is needed to make the tool happy) whenever the pressure to release/fix/whatever gets too high.
A regular peer review process or design, implementation, unit test, and results is what you want. The less ambiguous the standard you inspect to, the better. You should inspect to the standard C++ (or whatever language you use) best practice literature. Meyers, Sutter,...
There might be a place for pair programming for onboarding new/young people who don't know anything.
Sounds as bad as pair sex.
Confucius say, "Find worm in apple - bad. Find half a worm - worse."
I wouldn't know a better way to destroy productivity than having people pair programming in a public space. How is anyone supposed to concentrate on their work or their coding partner while listening to the conversations of everyone else in the room at the same time?
...pair debugging can be great. Two people looking at the same code and same test output at the same time can go back and forth designing tests and interpreting results really effectively, especially if the bug is at the intersection of two modules that you're reach responsible for. Of course, you don't spend all day every day debugging or something has gone very wrong wrong.
An interesting anagram of "BANACH TARSKI" is "BANACH TARSKI BANACH TARSKI"
Agree. We use both. Pair programming + 2 code reviews. Pairs test and review each others code. Then there is a final review. Works, but still have bugs.
There might be times when it's nice to have the second person helping pedal up a long hill. But you're certainly not going to double your speed or your stamina with two people on one bicycle.
Pair programming is like that. There are specific situations where it's useful, especially when you're dealing with a tricky algorithm or intricate business requirements. But much of the time, the second person is just dead weight.
First of all, there is no "best" practice on this. For some people, pair programming is a horrible experience, but for others it is a great one.
Second, not all code reviews are the same. Code reviews the go line by line are not productive - it is too close up a view. What is better is a design level discussion, where the programmer makes arguments for the things that matter about the feature, e.g., an argument that the feature extensible, or testable, or reliable, or secure, or maintainable, or whatever - that gives people a tangible set of criteria to discuss the design; and by "design" I don't mean some big up front architecture - I mean the actual as-built design, which the programmer should be able to explain and wither draw a diagram for or write pseudocode for - or else he/she is just a hack.
On one team, we did pair programming, but we didn't call it that. Instead we called it "real time code review." Turns out if you review while the code's being written it takes a LOT less time and gets the benefits of thorough reviews too.
Fuck if that makes sense. I do not see why I should follow anything religiously. Religion maybe, but even here you should use brain not to get abused by the truths dispensers trying to rob you or put their dick where it does not belong (and I say it not as a faith hater, neither I am religion hater). You use what suits you best and what you can use in a team. Pairs are good for some combinations of people, tasks and organisations. Sometimes other ways are better. A review of a document may or not be done (on top or not) - whatever is suitable. That religious following is what burnt scrum for me. This and abuse by managers that thought of it as a best method of getting rid of responsibility while getting bonus for removing project and system management. I think that is the same as what communists did - forced everybody to do the same and in the same way. It has to fail. Then as long as company achieves profits and can pay my salary I do not give a flying fuck - they tell me to do my job in the toilet then nobody will be able to use it - perfect, why should I care.
After all these years in software development and around, the only tool that I see as vital is rhetoric skill, used to tell managers what I think without abusing them verbally and in a way they can understand. They pays handsomely to use and is immense fun to watch them realize how big a cluster fuck they managed to cause. As with everything else (including pair programming and code review) use with care.
A pair of eyes, a pair of hands, a pair of monitors, a pair of ....
Knowledge is how to play a game, intelligence is how to win, wisdom is knowing what game to play.
> No, a review process is useless.
then
> You need actual code reviews for it to work,
I'm not sure what you're getting at.
The last two jobs I've done had a review process where all commits to the RCS resulted in a notification sent to the entire team, and was expected to be reviewed by two or more people.
One nice thing about this approach as well, better architecture. Not because of someone's review, but because the review process works far better when commits are small. Keeping commits small helped to improve modularity of our code, since 98% of the time we expect each commit to result in software that can be built (we did everything in our powers to avoid committing changes that would break a build).
I'm god, but it's a bit of a drag really...
Major Stockholm Syndrome case here...
In our organisation, we have teams of six people that work together on their sprint. QA staff are included in this team.
On major features, the team code reviews the feature together in a special session. Roles are assigned. The author is present, a reader (who is not the author) reads the code. There is an arbitrator who decides whether a raised issue gets fixed. This arbitrator role is rotated through the team on an inspection by inspection basis. Finally, there is a time keeper role who moves the conversation to a decision if one topic is debated for more than three minutes.
This process typically finds a humongous number of issues. It takes us about 4 hours of applied effort to discover a bug in pure functional testing. This process discovers bugs at a rate of 1.25 bugs per man hour of applied effort. So if you have five people in a room for one hour, you have applied 5 man hours. You'd expect to find 6-7 bugs. If you include all the stylistic coding standards bugs, this is typically 10-15 bugs per hour.
So while on the surface it looks expensive to have all those people in a room talking. The net result is that it tends to accelerate delivery because so many issues are removed from the software. Better still, the review occurs before functional testing begins. This means the QA staff on the team can direct their testing at the areas highlighted by the inspection process. This further improves quality
It's true that about 50% of the ossies are stylistic issues. But usually we get 1 or 2 bugs per session that present a serious malfunction in the program. The rest could be problems under some circumstances or minor faults.
Team reviews are vastly, vastly superior to pair-programming. There really is no contest.
"and was expected to be reviewed by two or more people" That is the crux there: it was expected to be reviewed. But was it reviewed? Who knows?
... I swear. Lean, SCRUM, XP, Agile, Waterfall, Kanban, Scrumban, TDD, BDD, Pair Programming, Code Review, User Stories... etc... etc... etc.
How about just be a responsible craftsman, understand the customer's requirements and needs, and implement your solution responsibly and with integrity? Whatever that means. If you need to pull someone in, then do it. If you don't, then don't. Christ, how complicated is this? It's one thing to be a junior developer and having to learn things. Fine. But an experienced software developer should not require constant canoodling to get their job done responsibly, with integrity, and with good quality. Is it really that hard?
I'm from a pretty old-school programming upbringing -- back when you were a "Programmer" or "Analyst" or "Programmer/Analyst". I'll tell you in those days... if a programmer demanded this kind of ridiculous hand-holding, canoodling, and process-implementation to get their job done... they would be fired. Plain and simple. This industry has become awash with process and tool zealots... while knowing the customer's needs be damned.
well, at Job 1 it was hit or miss, but at Job 2 we have a formal code review tool: https://www.reviewboard.org/
So yes, you do know who reviewed what, and whether or not they give their seal of approval.
now I'll admit that someone can just mark a review as fine without having read it, but as I said, nothing is perfect. Our process could certainly be improved more if we used a Github-style way of managing code (pull requests and all that) but since we use SVN and do all our work on the master branch it won't happen. I've fought for it but was the counter argument was that the team is not large enough to justify that kind of workflow. *shrug*
I'm god, but it's a bit of a drag really...
It's pretty easy to require formal sign-offs by reviewers.
In fact, that is how it's done in most mature IT organizations.
Slashdot social media options: AIM, ICQ, Yahoo, Jabber and Mobile Text. Why no MySpace?
Yeah, I use reviewboard as well. I like it, and I hate reviews, but it works.
I think the OP point was even with signoffs it doesn't mean any review was actually done.
Maybe not, but the reviewers' ass is on the line for bugs found that his review should have caught.
Skim over reviews too many times and it's bye bye.
Doing a bad job should have consequences.
Though I admit that might not work in smaller organisations.
Slashdot social media options: AIM, ICQ, Yahoo, Jabber and Mobile Text. Why no MySpace?
Unless you're training a junior programmer, pair programming is stupid. Having two competent programmers work together is wasting valuable time. Peer review is a necessity, but it can be pretty informal, shoving several people in a room together to go over code line by line is probably a waste of time to. Better to sent out emails with a list of files saying "Review these files and fill in a spreadsheet with your feedback", then have a meeting to discuss which feedback to ignore. Unfortunately, projects always start out with the intention to do reviews, but then reviews are the first thing to be abandoned when the schedule gets tight. Does pair programming prevent that?
I've abandoned my search for truth; now I'm just looking for some useful delusions.
In my organization you get one chance. If you miss a bug that should have been caught, you get fed into the shredder immediately. It keeps us on our tows. I mean TOES..aargghh
Case in point: I'm maintaining HP printer firmware. Do you have any idea how resistant they are to fixing bad design decisions made long ago by people that no longer work here? The code is the result of decades of band-aid fixes... and it shows.
I've abandoned my search for truth; now I'm just looking for some useful delusions.
Your plan, while certainly useful, does nothing to build good coding practices or encourage actually writing good code. It's not a code review or peer programming, it's the basics of working in a real team on a software project.
For women : pair programming.
For men : code reviews.
Pair sysadminning is something I have actually done, and that has its uses. The guy who knows most has the keyboard, and the other guy has one finger tracing the manual's instructions, as well as a notebook where he writes down what happens and why, especially including things that won't be in the terminal session script like "I'll correct that later" or "This value should work, let's check".
But nowadays with Infrastructure as Code and Network as Code and what have you, you check your sysadminning code into git and you do reviews on it!
Is this done in the first world (US, Canada, Europe) or only in Asia or the third world? I can understand willing to fork out the extra bucks to have two people do the job if you are getting them at a cut rate. But, it you are paying first world rates, who is going to pay double to get the output of a single developer?
I've spent nine years working in teams which religiously follow pair programming.
That is one clusterfuck of a professional experience. To me this tells me a lot of work in those years constituted code monkeying.
For starters, we are engineers. We are supposed to use general principles to solve real world non-trivial problems. This implies a need for adapting processes to issues at hand. Professional discretion is required when performing activities. Following something religiously gives me the notion of people going through the emotions by orders given from above. Good way for herding cats (or to control damage done by a workforce of mediocre skills.)
Now, seriously - who does just coding? As we mature in this industry, our roles takes us progressively away from the keyboard. We have to go meetings, we have to review requirements, we have to bug triage, we have to review documentation, we have to review test cases, we have to lend a hand to product support, we have to get engaged in deployment, we have to be engaged in planning, etc, etc.
Some times, all of that. Most of the time, some of that. And most of the activities mentioned above require participation with different teams. Participation in those activities gradually increase with time as we evolve in the profession.
So, how the hell can someone claim to have been doing 9 years of pair programming? What the hell happened to all the other activities *other than programming* that are required for developing non-trivial software?
9 years of pair programming = wasted opportunity in professional development.
Wow, just wow. Half the posts are "I don't like this thing so it must suck". I'm glad I got a degree in Software Engineering where we actually studied these things. Go out and read the research papers on all this stuff. Everything thing has it's pros and cons and are extremely beneficially in some situations while completely hindering in others. Self-taught programmers don't bother to learn these things and CS degrees don't teach it.
It doesn't matter what you like, it matters how well it'll work in your current situation and there are specific pros and cons you can look to instead of guessing.
Pair programming of new features = FAIL.
Pair programming with the guy who wrote the original code, when you find a bug: EPIC WIN.
Pair programming when creating new features: FAIL. Pair programming with the guy who wrote the original code when you find bugs: EPIC WIN
Typical Indian coding! Hell I've interviewed folks in India over the phone for my company and they have people in the background COACHING them on the answers! I do code reviews for work contract works from India have done - a lot of it looks like they squirted it out their arse! I'm not impressed! This article is a PURE JOKE! They "pair program" because one by themselves couldn't code a simple application if they tried. I've seen it in action! We have three conference rooms full of them - you will see 3-4 of them crowded around one laptop jabbering away pointing at the screen and arguing. And they still push broken code to production. That is why we now have FULL code reviews of EVERYTHING they do. Can't tell you how much $$$ it's cost the company.......
Even if you would think pair programming makes sense (I don't, but it might be cultural as some have claimed), they cannot replace code reviews.
Both "pair programmers" are following the same thought process and may fall in the same trap.
A fresh look, by someone who wasn't involved in coding is always required.
As no good coders work there (they have all gone to greener pastures), maybe you can bring code quality there up from "complete and utter crap" to "complete crap". Not sure this is worthwhile doing though. (Yes, I have reviewed code produced there. Several times. The sheer level of non-understanding you can find in there is staggering.)
Most ACs are not even worth the keystrokes to insult them. Be generically insulted by this and ignored otherwise.
Pair programming requires one to continuously socialize with someone nonstop for entire working day, something that most people, let alone most software engineers, can not do while maintaining concentration, creativity or sanity.
Code reviews encourage filibusters when someone keeps adding to the comment thread just because they personally want something done differently and others find it rude to approve the change and curtail discussion.
What works well is running lint and writing good tests. Then if someone is motivated enough to address their grievances with your code, they can clean it up on their own time.
Why vs? Neither of them is worth crap for actually improving code. Room and incentives to refactor and simplify plus unit tests (of functionality and not deeply dependent on exact implementation) does a lot more good.
The only way I could get pair programming to work was to get in a room with a projector, and give the one developer the control of the keyboard. The other developer gives instructions and directs the flow (while also writing things out on the whiteboard to teach the other). If some changes/fixes take a lot of time, this lets the both developers work on their own laptops.
This worked irrespective of whether the junior/senior dev was projecting. It is more hands on for the driver, while giving more flexibility to the partner to do their thing. Traditionally the junior dev controls the keyboard, while the senior dev is like a mentor - they don't take the wheel from junior, but they don't need to be wasting time while code edits are happening.
It's s reference to the movie Swordfish.
It's disheartening to see people with entrenched positions lash out the minute something new threatens their illusory bubble of safety. The reactions are visceral yet I wonder how many have actually tried pair programming?
It's really this simple: if you think peer review has value then pair programming is simply just-in-time peer review. Furthermore, by virtue of the fact that it is just-in-time, it has one big advantage over traditional code review: the reviewer is immediately invested in the process. In other words, they can't relegate their code review to the background.
Here's how it works: two people sit at a pairing station which has a monitor, keyboard, and mouse for each developer connected to the same workstation. The programmers take turns "at the wheel". The programmer that is not "driving" is reading the other programmers code, catching typos before they turn into hours of debugging, acting as a sounding board for design decisions, providing a second opinion or consensus when making tough decisions. Pairs rotate at intervals.
Yes, your team will write less code overall, but 1) the quality and the maintainability of the code improves significantly 2) the confidence in the code increases 3) the team benefits from shared knowledge.
Pair programming is not about how you feel it's about better teamwork. Yeah, a senior developer will feel that pairing with a junior developer slows them down, but those justifications are selfish. If I see a programmer making a mistake or tediously performing the same repetitive task shouldn't I teach them a better way?
Leave your ego at the door and accept that 1) you don't know everything; therefore, you will learn new things working with others 2) sharing your knowledge takes time but is a force multiplier 3) sometimes trying something different can have a positive effect 4) working in a closet with your headphones blasting "Dual Core" without team interaction may be heaven for you but its likely detrimental to others.
Grow up people.
It mostly depends on your team.
Pair-programming is hard, code reviews are pretty easy. Pair-programming increases team cohesion, but it to be properly done or it becomes than useless.
There is the driver, who writes the code and the observer who review and thinks more about the general design. There should be a frequent alternance, unless there is a vast experience gap between the two people, then the less experimented drives more (to avoid the keyboard domination antipattern).
Pair-programming should only happen on 60% to 75% of the day, because it really washes you out.
It works best with small teams (4 to 6 people)
Pair programming may not work with your team, but when it does, it brings a great team cohesion and a great code quality and better design. And as one who practiced it for 5 years (but not these days, in another team), it was great!
Stupidity is the root of all evil.
I've personally written half a million lines of high quality code. Not to create code, nor to reinvent wheels; rather, I regard each line of code as something I have spent to get a job done, and which adds to debugging and long-term maintenance loads. And I've supported/debugged/maintained a lot more software. Much of this was done working with a few other hot programmers. We used a mix of pair programming and code reviews, and you'd have had to pry these tools from our cold, dead fingers before we would have stopped using them. Why? We were deeply and personally committed to being as productive as possible, and getting more than one set of eyes on the code sped us up.
We thought about it like this: the stuff you understand how to do is fast and easy, and takes just a few percent of your time at the computer. It's the other things, the ones you have to think about, that slow you down. Empirically, the chances are strong that your partner will see a way how to get past something that stumps you. and vice versa; the likelihood of you both being stuck on the same problem is small. So pair programming resulted in much greater productivity than the same programmers working apart could achieve. The same benefits accrued to the overall design of the software, not just its implementation: we produced not just a lot more, but a lot better code.
Your mileage may vary - but having seen pair programming and code reviews work over and over, it's hard to give much credit to people who haven't tried these techniques talk about how they can't work.