Code Review - What Medium?

[ghenry]

Dear Master Monks,

How do you review your code?

Printing it out or on the screen?

Just out of interest, that's all.

Gavin.

[kirbyk]

Through unit testing.

While some other languages get you in bad habits of writing massive monolithic chunks of code before you try to (compile and) run it, Perl is ideally suited to running frequent tests and writing tests scripts to hit the code. My eyes will gloss over things that actually running it won't. It's not a great use of time for me to read my own code again - but writing a re-usable test to verify it works, and it still works after things change, that's where it's at.

Alternate answer: on my coworker's screen. While I'm not likely to see bugs or a better approach in my own code, someone else sure is. It's enormously instructive to have an equally experienced or better programmer review your code, and one of the best ways to learn new techniques.

[halley]

Unit testing is good, however...

Correct behavior is not the only thing that a code review should cover. Performance, documentation, readability, maintainability, and whether or not you're using personal idioms that are not typical for your peers. Remember, the source code is ultimately for the human's benefit at least as much as it is for the computer's benefit.

[Transient]

It all depends on two things:

1. How big the code is.
2. How nice of a day it is outside.

For me, if it meets both of these criteria, I print them out, grab a pen, a can of chewing tobacco and head outside to enjoy two of my favorite things: code and nature.

For me, sometimes it's easier to get out of the computer for a bit... I'd say personal preference

[jhourcle]

I like pens, but I use lots of pens, as I tend to make notes in more than just one color.

For me, it depends more on just what it is that I'm trying to review than anything else. I try to keep to the whole paperless office thing, but I find that printouts are useful when trying to trace the logic flow through large programs, especially those that I didn't write myself.

I tend to use at least three colors -- typically, I'll go through once, and try to reduce out that stuff that's not important (scratch it out in black, as I find it's less obtrusive), and make marks in the margin using blue to track where important sections are, and red wherever I think there might be problems.

There's not really any firm rules on the colors, though... if I have a highlighter, or a green pen, I might use some other marks of interest. Sometimes, I'll just use question marks to mark sections that I'm unsure of, and arrows to mark other stuff I want to come back to review again.

I do agree that sometimes it's easier to look at paper copies, rather than staring at a computer screen... especially when you get to that point where your mind is swimming, and the letters seem like they're trying to crawl about the screen.

I don't tend to go outside, as I also make heavy use a white board to keep track of entering/exiting objects, subroutines, or whatever. (and I don't chew tobacco... I much prefer large doses of sugar and caffeine)

[tlm]

Both :-) . Seriously: I do a presentation to be projected from my laptop, and hand out hardcopies of the code to be reviewed. People usually thank me for it; it is easier for the audience to mark their copies of the code; and it is easier for me to talk about the code if I can point to the projection.

[Tanktalus]

Via VNC.

Seriously - my team is about 3200km from me. So we hook up via VNC over VPN to a common desktop somewhere (a user set up especially for code reviews which has no other permissions on that machine or network), bring the code up in our favourite syntax-highlighting editor, with a terminal open for using "diff -u", and go over the pieces while on the phone.

[GrandFather]

On screen. I edit that way. I format it to look good that way. It's easy to navigate and search and it has syntax colour highlighting. It's easy on trees and land fills.

The down side is that it is not so good to scribble on the screen with a pen!

[etcshadow]

When I see code printed out on paper, my brain falls out through my ear. But hey, that's just me.

[sock]

9 times out of 10 I use `svn diff`. Nearly every piece of code that I run though usually has a test unit associated with it. Maybe its just habbit but its worked well for me so far.

[jhourcle]

A number of folks ( gawatkins, Tanktalus, GrandFather ) have mentioned that part of their reason for prefering on-screen review is that it provides them with syntax highlighting.

This isn't an issue for me, as I have access to a color printer, and a text editor that will print its syntax highlighting, not only display it on the screen. (and even when I print to the non-color printer, I can make out differences in shades, although if I try to print pages 2-up at 9pt, some of the 'colors' become illegible from dithering)

Others ( kirbyk, tlm, and anonymous ) have mentioned working with others, or in a team during the review. As I'm two time zones away from the next closest programmer on my current project, the best I have for collaboration is IM, and pushing files around through CVS and/or email.

I think my big question in this whole thing is what to do people consider to be 'review' ...

Much of what people are talking about (actively making changes, or making sure that the changes have the desired results), I would consider to be debugging, or in some degards, tuning. When I hear 'review', I think of a much more higher level thing -- attempting to look over the logical flow of code, and trying to understand it, often to determine how we can refactor it. Sometimes, it's trying to make the code more modular, or trying to remove redundancies. Other times, it's just me trying to understand how someone else wrote their code. (eg, trying to understand how SOAP::Lite works, so I can override the serializer to do what I needed).

So, are we all even discusing the same sorts of activities here?

[displeaser]

Hi,

Like a few of the others I review straight from the screen. If it's well formatted it's a pleasure to do it.

If I have some code that is been a bit "boisterous", I will print it out and read it on my hours comute home (pretty much the only time I'm not plugged into something ;-)

Displeaser

[Anonymous Monk]

Dear Gavin, Our team prefers to print it on paper and review it line-by-line. The only reason to do it, we can comfortably sit in conference room and discuss every single line :-). However PC is used for testing the review suggestions only in case reviewer and programmer do NOT technically agree on certain review comments. - Sandeep

[Gekitsuu]

I don't consider myself a master by any stretch but I prefer to do everything in vim; that way I can immediately run the code and see if the changes I'm making are having the desired results. Transient said they like to take the code outside with them and I have to agree that is nice but if the weather is agreeable then I usually just lug my iBook out with me. The further away from noise I can get myself the better :)

[steelrose]

Generally, I prefer the screen, unless i'm inside a really large loop. For example, I interited a program that has a nested if statement that is several hundred lines of code. I've printed that out and refer to it when dealing with that program (which of course, I am rewriting because it can be done much simpler).

[gawatkins]

Gavin



I usually prefer a good text editor with syntax highlighting for review, the highlighting seems to help weed out the typos a little quicker. But sometimes the change of looking at a print out will help me find the not so obvious bugs.


Greg W.

[PetaMem]

used to pretty-print perl source on paper and then going with a thin red pencil over it. Dropped that habbit, because often I discovered repeating errors and ended up correcting only the first few pages.

Now code review happens on screen, with syntax colouring, perltidy and cross-reference tools available

[kscaldef]

I find that on screen tends to work for "the little stuff". However, for understanding the large scale structure of a chunk of code, I usually print it out (using something like enscript or a2ps that can do syntax highlighting in print). Particularly if I didn't write the code myself, I find printing it out and marking it up with comments and corrections invaluable.

[poqui]

In all the myriad ways of reviewing code that I have gone through (including not doing it), my favorite is the formal code review we did at MCI.

The author would publish the code (at that time it was C) to a central spot. Then the review facilitator (usuallly someone from the QA or PM group) would schedule the review, and point all the reviewers to the central code. The reviewers were to be given at least 3 days to review the code and prepare their questions/ objections. On the day of the review, the facilitator would poll the invitees to see if they had completed their reviews, and if enough had (at least 3 developers besides the author) the review went on as scheduled.

At the review, an overhead with the code in question, as well as printouts of the code were provided, and the facilitator asked if there were any questions or objections to the code. The questions and the objections were recorded and the author was given a change to explain or rebut. At the end of the review, a go/no-go decision for the code to proceed to implementation was requested, and if consensus was not reached, the date for the next review was discussed.

This was a huge improvement over the previous way we had done them, which was for a bunch of people to get together in a conference room and have the code read to them line by line.

[nothingmuch]

I've recently had to read several thousands of lines of C++.

Doxygen and cscope made me very very very happy. It beat paper even on screen because I can read the code in the doxygen format, view weird hierarchies (class, file, function calls), and when I think I know where I'm going to investigate vim with cscope is an easy way to help your brain emulate a callstack.

I haven't ever needed to go through this process at such a scale with Perl code, but i will be searching for such capabilities when I do.

[arc_of_descent]

On the screen. In my favorite text editor. Also, I don't have a schedule where I do a code review. I would have to say its an ongoing process. Maybe I have to learn more!

[Anonymous Monk]

On the screen, if I'm at my desk. On paper, if I'm on the subway.
--
Ytrew