As I alluded to in my last post (which I will becorrecting shortly), I no longer work for Google. I still haven'tdecided quite where I'm going to wind up - I've got a couple ofexcellent offers to choose between. But in the interim, since I'm nottechnically employed by anyone, I thought I'd do a bit of writing aboutsome professional things that are interesting, but that might havecaused tension with coworkers or management.
Google is a really cool company. And they've done some reallyamazing things - both outside the company, where users can see it, andinside the company. There are a couple of things about the inside thataren't confidential, but which also haven't been discussed all thatwidely on the outside. That's what I want to talk about.
The biggest thing that makes Google's code so good is simple: codereview. That's not specific to Google - it's widely recognized as a goodidea, and a lot of people do it. But I've never seen another largecompany where it was such a universal. At Google, no code, for any product, for any project, gets checked in until it gets a positive review.
Everyone should do this. And I don't just mean informally:this should really be a universal rule of serious software development.Not just product code - everything. It's not that much work, and itmakes a huge difference.
What do you get out of code review?
There's the obvious: having a second set of eyes look over codebefore it gets checked in catches bugs. This is the most widely cited,widely recognized benefit of code review. But in my experience, it's theleast valuable one. People do find bugs in codereview. But the overwhelming majority of bugs that are caught in codereview are, frankly, trivial bugs which would have taken the author acouple of minutes to find. The bugs that actually take time to finddon't get caught in review.
The biggest advantage of code review is purely social. If you're programming and you knowthat your coworkers are going to look at your code, you programdifferently. You'll write code that's neater, better documented, andbetter organized -- because you'll know that people who'sopinions you care about will be looking at your code. Without review,you know that people will look at code eventually. But because it's notimmediate, it doesn't have the same sense of urgency, and it doesn'thave the same feeling of personal judgement.
There's one more big benefit. Code reviews spread knowledge.In a lot of development groups, each person has a core component thatthey're responsible for, and each person is very focused on their owncomponent. As long as their coworkers components don't break their code, they don't look at it. The effect of this is that for each component, only one person has any familiarity with the code. If that person takes time off or - god forbid - leaves the company, no one knows anything about it. With code review, you have at least twopeople who are familiar with code - the author, and the reviewer. Thereviewer doesn't know as much about the code as the author - but they'refamiliar with the design and the structure of it, which is incrediblyvaluable.
Of course, nothing is every completely simple. From my experience,it takes some time before you get good at reviewing code. There are somepitfalls that I've seen that cause a lot of trouble - and since theycome up particularly frequently among inexperienced reviewers, they givepeople trying code reviews a bad experience, and so become a majorbarrier to adopting code review as a practice.
The biggest rule is that the point of code review is to findproblems in code before it gets committed - what you're looking for is correctness. The most common mistake in code review - the mistake that everyone makes when they're new to it - is judging code by whether it's what the reviewer would have written.
Given a problem, there are usually a dozen different ways to solveit. Andgiven a solution, there's a million ways to render it as code. Asa reviewer, your job isn't to make sure that the code is what you would have written - because it won't be. Your job as a reviewer of a piece of code is to make sure that the code as written by its author is correct. When this rule gets broken, you end up with hard feelings and frustration all around - which isn't a good thing.
The thing is, this is such a thoroughly natural mistake to make. If you're a programmer, when you look at a problem, you can see a solution - and you think of what you've seen as the solution. But it isn't - and to be a good reviewer, you need to get that.
The second major pitfall of review is that people feel obligated to say something. You know that the author spent a lot of time and effort working on the code - shouldn't you say something?
No, you shouldn't.
There is never anything wrong with just saying "Yup, looks good". If you constantly go hunting to try to find somethingto criticize, then all that you accomplish is to wreck your owncredibility. When you repeatedly make things to criticize just to findsomething to say, then the people who's code you review will learn thatwhen you say something, that you're just saying it to fill the silence.Your comments won't be taken seriously.
Third is speed. You shouldn't rush through a code review - but also, you need to do it promptly. Your coworkers are waitingfor you. If you and your coworkers aren't willing to take the time toget reviews done, and done quickly, then people are going to getfrustrated, and code review is just going to cause frustration. It mayseem like it's an interruption to drop things to do a review. Itshouldn't be. You don't need to drop everything the moment someone asksyou to do a review. But within a couple of hours, you will takea break from what you're doing - to get a drink, to go to the bathroom,to talk a walk. When you get back from that, you can do the review andget it done. If you do, then no one will every be left hanging for along time waiting on you.
聯(lián)系客服
