[Chicago-talk] code review discussion

[David Mertens]

Hey folks -

I've never been involved in a code review so last night's discussion
was pretty informative. In general we seemed to have a good time.
However I observed last night that our final code review seemed a
little tense. I have a few observations about it that may explain why
it differed from the first few reviews:

1) As was pointed out last night (by John?), we had been sitting in
the dark drinking beer and looking at computer screens for two hours.
That's a long time to stay sensitive and patient. Take away point:
keep code reviews short (and on time, though that wasn't really an
issue last night).

2) A lot of people took issue with the architecture of the last
speaker's solution to his problem. (Sorry, I don't remember your
name.) This was in sharp contrast to the previous code reviews in
which we mostly nit-picked on details. Attacking somebody's
architecture is much more offensive than picking on details; ideally
the architecture of the solution would have been discussed in a
previous meeting, at which point we would be reviewing the code for
implementation.

3) That having been said, the speaker did a very poor job explaining
why he was using such a huge hammer for such a simple problem. He had
a good reason, of course, but he spent so little time motivating the
solution that all I remembered was 'code reuse,' and I was left
scratching my head as to why this was better than writing a simple
function. (My ignorance about Moose was a further barrier for me, but
hopefully that would not be a problem for his team.) Obviously the
take-away point here is that, in your preparations for your own code
reviews, you must motivate why you chose the particular solution that
you chose. (None of the speakers prepared for this, so I'm not saying
they did a bad job.)

4) As with all reviews, code reviews should focus on what the
programmer is doing *well*, with only a few items for improvement.
Take away: when you're presenting your own code, start by showing off
your good stuff, and then move on to any potential problem code. When
you're reviewing somebody else's code, work really hard initially to
find what's good about it, what the programmer did well. Frankly, I
thought that the implementation of the code was excellent, it just
needed some documentation and examples.

Overall, the evening was a good one and I'm sorry I wasn't able to go
out afterwards. Hopefully next time!

David