Monday, May 4, 2009

Code Review without Standards

Code review. Those words get thrown around a lot where I work. I haven't actually been a part of, or even witnessed one, but people at least talk about them. Another colleague who is new here like me said he really wants code reviews in place here. He had that at his previous job and really misses having someone review his work and make suggestions.

One issue that this brings up in this particular situation is standards. This particular team I'm on now hasn't really got any standards in the way they develop. Standard IDE? No. We've got devs in jdeveloper, intellij, eclipse, and for a brief moment netbeans (I had to try ;]). This kind of thing seems like a freedom, but to newcomers and average developers this can be a bit taxing at times plus it leads to little problems that like to edge their way into the mix. How does a person go about ramping up onto the team? If you sit with person-a on Monday and they use eclipse and setup their environment with configuration-b, but on Tuesday you had to work with person-b with jdeveloper and configuration-0 then how productive are you going to feel on Wednesday when you get to setup your own machine? How does one go about documenting a developer ramp-up tutorial? Should you do one for each environment?

That is just a glimpse the ice burg. There are other things like standard code formatting and 3rd party library use vs. reinventing the wheel (how many times should you write a string utility class?) This is nothing new. Every software team across the globe has had to deal with this kind of thing at one point or another. So let me address my questions:

  • Can you implement code reviews without first having in place some form of standardization?
  • Could code review be a platform from which to define standards?
  • Should a standard IDE be encouraged? I shy away from enforcement.
  • For those with prior experience in this: How should a group approach implementing standards and reviews? Hoping to avoid making everyone mad.
What say you?

Until next time
Les

3 comments:

Unknown said...

Yes, you can implement peer code review without first implementing coding standards, but watch out... you might end up with reviews that are just discussions about *what* the coding standards should be. And that's not a very efficient use of time.

In other words, if a team can agree on coding standards (naming conventions, white space usage, etc.) then you can use static analysis tools to check the code for compliance with those standards. That allows you to save peer code reviews for finding the types of problems that static analysis tools cannot find: are the unit tests complete? is the algorithm correct? will it scale? etc.

With respect to avoiding making everyone mad, check out this blog entry: http://blog.smartbear.com/the_smartbear_blog/2009/03/tips-for-developers-social-effects-of-code-review-part-3.html. More general tips available here: http://smartbear.com/codecollab-white-paper.php

Forrest said...

I think a standard IDE/toolset is a good idea, but as you say probably shouldn't be enforced. That just leads to potentially alienating some of your best people.

If your department has enough newcomers flowing in, maybe it's time to develop a reference implementation. That would be a good way to introduce new developers to the standard toolset and the general architecture of your application.

WonderBread said...

What a great blog! I like gsporar's comment about naming conventions, white space usage. I believe he's talking about formatting. As long as you have a standard formatting for the code, I believe that would remedy some of the pain of different IDE's. Standardizing an IDE is also a good idea (as forrest said). It may alienate some people but your best people are flexible anyway so I believe they could roll with the punches there. Maybe see which IDE is the most popular on your team and go with that one. I would also think that TDD would help the scalable algorithm issue that was brought up by gsporar. Maybe a good reference implementation with good tests behind it would help standardize some things. Breaking the news on the IDE standard would be touchy.