Code Review Mashup

We ran what I’m going to refer to as a code review mashup early this week on a project. The preparation looked like:

  • Setup a meeting time for a 2 hour block by the tech lead.
  • Setup the code review in Crucible selecting four classes written by three different developers including the lead about one day ahead of time.
  • Added the three developers and myself as reviewers.
  • Planned to walk through the code on an overhead and do more of an inspection.
  • The morning of the review I took about 45 minutes to review 2.2 classes out of four and added about 15 comments to the code using Crucible.

I led/facilitated the review by going through all of my comments. Since I was the only one who had spent any real time prepping, the other developers only caught a few extra things. I think the big takeaway is everyone needs to review the code ahead of time. The last two reviews I’ve been in too much of a lead role, and not spending the bulk of my day coding I miss things other people can catch.

The review meeting ran much more like a code inspection or walk-through, essentially a code review inspection mashup. I’m not sure mixing the two styles really works all that effectively.

Still for about 8 hours of investment across four people we caught:

  • A decent amount of coding convention misses, mostly naming conventions such as: <div class="codecolorer-container text vibrant overflow-off" style="overflow:auto;white-space:nowrap;">
    1
    findBySSN

    </div>

    needs to be

    1
    findBySsn
  • A design debate over whether to break up a fairly large JSF backing bean which is handling 5 physical pages into five separate backing beans or keep it in one. (We didn’t resolve this for now, but it is a future topic to consider.)
  • Whether to use DTOs when using Hibernate or just use the Hibernate mapped model classes. We talked about the potential ease of not using DTOs, and the potential extra coupling of using Hibernate mapped models through the application layers. For now we’re using the DTOs.

Standards, Guidelines, and Practices

My BS radar goes off when conversations turn to standards and best practices. I find James Bach’s argument quite convincing especially his point about industry best practices:

  • “This practice represents a consensus among industry leaders.” No it doesn’t. You’re just making that up. Industry leaders aren’t able to agree on anything much of substance. I know this because I am an industry leader (based on the fact that some people have said so), and other leaders more often disagree with me instead of obeying my commands. I am influential within a certain clique. The software testing industry is made up of many such tiny cliques, each with its own terminology and values. </ul> </blockquote>

Standards are far too often used as hammers to enforce an individual’s ideas. They can quickly stifle creativity and lead to ludicrous decisions like using EJB’s for every project because that’s the industry best practice we’re following.

In Peopleware the alternate recommendation is to tend towards a “gently guided convergence” over hammering people with declared best practices. There ideas are:

  • Training: People do what they know how to do. If you given them all a common core of methods, they will tend to use those methods.
  • Tools: A few automated aids for modeling, design, implementation, and test will get you more convergence of method then all the statutes you can pass.
  • Peer Review: In organizations where there are active peer review mechanisms (quality circles, walkthroughs, inspections, technology fairs), there is a natural tendency toward convergence.

— pg. 118 Peopleware 2nd Edition

Not a bad set of ideas, even if I’m not sure what a quality circle is.

Enterprise Software A Dangerous Choice

I’ve worked in several IT shops where various CIOs sold the business on how some great new Enterprise technology was going to make IT better, faster, and more successful. I’ve never seen it work out all that well.

In one particularly absurd environment they went through at least three completely different Enterprise technologies in the space of about five years.

  • First there was Rapid Application development with Powerbuilder.
  • Next up was Java and J2EE.
  • Then it was .NET that was going to save the day.

The end result was predictable. The large centralized IT department could barely deliver an application because they were always on a learning curve. The business picked up on this. Multiple CIOs and middle managers were demoted or asked to be successful elsewhere. The central IT department was largely broken up and distributed out to individual business units. There is very little reuse since each unit does it’s own thing, but at least they feel like they have some control.

Group The Strong

David Anderson recently posted about his reading of Richard Farson’s Management of the Absurd.

Farson contends that in flat structures with self-organizing teams the team member will weed out the the stronger member. In hierarchical structures people will do the Darwinian approach and weed out the weaker members.

As David Anderson notes this is obviously a problem in an Agile context. I’ve always followed the opposite approach. My two rules are:

  • At least two developers per project. One developer is just too risky and doesn’t share knowledge.
  • If possible at least one tech lead per project who’s top responsibility is to bring up the rest of the team. As developers grow in experience this is becoming easier to cover.

I haven’t seen developers go after the stronger team members on our Scrum projects. I’ve actually seen more of the Darwinian behavior. What I’m really looking forward too is I’m able to staff whole project teams soon with strong experienced developers. I just need to convince senior management not to swap out 80% of the technologies everyone’s mastered again and start much of the learning process over.

As an old fan of Survivor I can see that given the context of a million dollar prize cooperation tends to lead to voting off the stronger members first where possible, because they’re real threats. I just don’t think that’s how Agile projects usually work.

Considering contrarian ideas helps expand your vision, so it might be worth picking up the book. Some crazy ideas really work better than expected.

Standup Time Boxed By Lunch

Typically daily Scrums are held as early in the morning as reasonable or later in the afternoon. Due to some strange circumstances the intranet portal project I’m Scrum Master on has a daily Scrum at 11:15, right before lunch. At our retrospective early this week I asked if we should move it to another time or at least to say 11:00. The universal response was no, don’t move it.

The reasoning across the team was that by having it right before lunch it forced the meeting to fit into the 15 minute window and it cut down on the number of followup meetings held just after the Scrum. They liked having that added pressure to get the meeting done before running off to lunch. Strange, but it’s up the team to self-organize.