Slashdot Mirror
Are You Too Good For Code Reviews?
theodp writes "Why do some programmers,' asks Chris Hemedinger, 'place little value on code reviews?' This apparently includes even Programming Greats like Ken 'C' Thompson, who quipped, 'we were all pretty good coders' when asked about the importance of code reviews in his work. Hemedinger, on the other hand, subscribes to the school of thought that peer code reviews are Things Everyone Should Do. Not only do reviews keep you on your toes, Hemedinger says, they also 'improve quality, ensure continuity, and keep it fresh. Who can argue against that?'"
And some of that needs to testers who are NOT coders or people who are not mainly doing programming.
I have worked in places with and without code reviews. Yes code reviews can be painful especially if you work with some douchebags. But overall getting new eyeballs looking at your code often brings up things you hadn't thought about, etc. It doesn't make you or the other people bad, it's just how it is. Drop the ego and take some constructive feedback.
Hold up, wait a minute, let me put some pimpin in it
Perhaps peer code reviews were less important 30 years ago. The constraints around running code were much tighter. There were fewer layers in the stack where bugs could lurk. Memory was scarce, instruction sets were limited. Computer applications were like my 1973 AMC Gremlin: fewer moving parts, so it was easier to see which parts weren't working correctly and predict when they would fail.
Welcome to the industry, Chris. This might have been the funniest thing I read all week.
I'm too busy for code reviews.
Development schedules are almost always ridiculously compressed. There's no time in the schedule for code reviews. Just like there's no time for thorough testing.
But the software only has to be 'good enough' for people to buy it, so there's no ammunition for developers to use to get a better schedule.
The problem is that the need for code reviews is driven by lax, sloppy developers who don't see regression testing as a requirement, and who foist crappy, untested code that, in many cases, they haven't even tested.
As a consulting exec (experience side) who oversees software delivery I can't even begin to express the stunning crap that I see developers submit for "qa/review". Crap that doesn't even WORK correctly in the first place is submitted for testing, with the QA feedback often "Does not work". Aside from the hours of UE and QA resources this burns with useless testing, it highlights what I think is both an increasing lack of accountability and a lack of professionalism within the development community in general.
What's driving this I have no idea... less formal CS training? Looser languages? Web-centric apps? Lower end standards? Higher demand = more crappy resources? Whatever it is I'm seeing it everywhere, and it's driving me nuts. The lack of an appreciate for regression testing is absolutely insane... code reviews are just symptomatic of a larger problem, which is a lack of quality and skill.
-rt
I tend to use code reviews as a method for showing other team members the work that I have done so they will know about the changes. I work with a large number of geographically disperse people and having 1:1 conversations about every little change can be tiresome and wasteful. Code reviews are an excellent way to get around this problem.
Another bonus for code reviews, along the same lines - is in teaching people new to a platform about the kinds of bugs to expect when working on the platform.
It also keeps me honest, did I just add a debug, or do I also need to add trace? If I forgot the trace, code review will pick that up.
The attitude that the ends justify the means in software engineering is a problem. Software isn't a write once and forget it product nor is usually written by one person. The more people that need to work on a piece of code the more that code needs to adhere to the groups coding standards. I would highly suggest that everyone who writes code treat it as if the code is also a part of the product. Avoid shortcuts, be neat, be concise, and look at it as if you weren't the one who wrote it.
automated testing is primarily for regression testing anyway. GUI / user experience 'testing' is usually after that point, since 'GUI' is just the interface and not the back end logic. Working but looking bad is inherently preferable to looking good but not working....
People in cars cause accidents....accidents in cars cause people
As for testing - that's a later stage in the process of development.
Testing and code reviews should occur at the earliest possible moment and be integrated throughout development. Bugs cost less when they are found earlier.
Colleague of mine used to have the following sig: "Write code as if the person who will be maintaining it is a violent psychopath who knows where you live."
Because if it's written with an eye only towards effectiveness, and not towards standards and readability by others; then by the standards of any project larger than "A script I use to clean up my home directory" it's crappy. He's being kind of an arrogant ass about it, but his overall point is completely valid. If you quit or get run over by a bus tomorrow, and I (for a value of "I" that indicates a competent peer familiar with your company's standards and the language in question) can't pick up your project/module and continue development/support then it's not "good" code no matter well it works. That doesn't mean you can't be creative and awesome in your implementation, it just means you have to write the shit so someone else can read it; and in a pinch finish/fix it. That's what standards are for.
I don't need a million points of light, just two points of multi-mode fiber and a 10 Gig-E router.
I've caught bugs in code review that would never in a million years have been caught by testing. The customer would've encountered them sporadically, and in situations in which they'd come to rely on the broken behavior working implicitly. The would've submitted a bug report and we would've been unable to replicate the problem in house. The customer's opinion of us would be lowered, and we'd have all these bug reports in our system that nobody could figure out.
That is, until someone reviewed the code and discovered the bug.
So, basically what you're doing is robbing Peter to pay Paul. By not doing the reviews you end up with less time in the long run, not more.
Need a Python, C++, Unix, Linux develop
My goal, as a developer, is to make sure QA never catches any bugs. When they find a bug, I'm embarrassed, as I should be. Developers who have a purposeful strategy of waiting for QA to find their bugs should be fired.
Need a Python, C++, Unix, Linux develop
He's being kind of an arrogant ass about it,
I earned my Arrogant Ass Badge honestly through hard work and paying my dues, so I reserve the right to pull it out on occasion. :)
Sometimes it's best to just let stupid people be stupid.
Finding a bug in your own code costs one of unit of work. QA finding a bug in your work costs 10 units of work. Having a customer find a bug in your code costs 100 units of work. Doing code reviews increases efficiency. Plus, you get to both teach and learn from your peers.
That's all nice and dandy, but the problem is that in reality it still makes sense sometimes to produce and sell buggy software. This is so for a variety of reasons. Amongst them the fact that actually pulling off a business is a very multifacetic thing, and the product is only one of many aspects. In many cases, the cost of making a good product is simply prohibitive when viewed in the grand scheme of things.
Reality is messy.