greetings all!

Tue Apr 13 08:06:13 CDT 1999

in our last meeting, it was decided that i would share what i know about
code reviews. the others thought i might be able to pass along some of the
experience i have gleened. we'll see.

i want to keep things simple so we will be reviewing a section of Perl code
that prints out a calendar feature of a simple app that i wrote. to give you
some context, check out:

http://www.stevenhenderson.com/demo

to give you context of the code, i have attached:

1. review.txt -- actual code snippet to be reviewed
2. demo.zip -- full source code to demo app

if you check out the demo app at the URL above, this function is responsible
for printing out the 'month' table located in the bottom frame. this was
written some time ago and is likely riddled with bugs, should make for a
good review!

please print out the review.txt and bring it with you. i will try and
remember to bring extra copies, but i don't have a printer so i gotta run to
kinkos.

i have been looking through my old documents and could not find a short and
concise description of this process, so i will just wing it for your
benefit below.

CODE REVIEW DEFINED:
a code review is a short (always less than 1 hour) meeting involving
developers interested in improving programming code quality. it can be
viewed as a 'peer review' your code is analyzed by others in your
development group as well as other programmers outside your group.

WHY:
the goals for the code review is simple: to improve code quality and avoid
potential bugs. the main purpose is to find bugs in the code you are
reviewing, but other benefits include learning logic & process strategies
from people smarter than yourself.

WHO:
reviewers: anyone interested in the code review. generally they are part of
your development team and sooner or later integrate with your module. a
'floater', or person not involved with your project should be invited as
they often can see the forrest through the trees and can bring a new
perspective to the table.

reviewie: person hosting the meeting and who's code is under scrutiny.

thus there must be at least two developers attending, and as many as 10.
however, be cautioned that any number of five is generally unpreductive as
it is too hard to keep the review on track.

WHAT:
everyone invited should be provided both electronic and hard copy of the
code to be reviewed. this printed hardcopy should be brought to the code
review and should be formatted in a standard way (depends on the company).
the code review begins with a quick q&a and then proceeds to pick the code
apart line-by-line.

the meeting proceeds as every line is 'checked off'; ie: everyone agrees
that the line is ok and no potential problems are lurking.

very important: these sessions should move along as quickly as possible and
NOT focus on stupid problems such as:
    . misspelled comments.
    . formatting style (should be standardize within the company anyway).
    . variable name choice (unless its a bug or confusing)

remember that you only have 1 hour to get through the entire code segment.
additionally, it is equally important that the reviewie does not get
defensive when people criticize thier code. this is harder than it sounds
when you are under the microscope, but egos must be left at the door!

WHEN:
a code review must take place before any ship-critical code is complete.
generally they should occur as often as peoples schedules will allow. i
found them much more benificial to have several smaller code reviews more
often than wait to have a huge monolithic block of code to be reviewed.

(note: in my last team, we had a conf. room scheduled every thursday at
1:00pm. there were 5 of us and we would alternate a code review every week.
this kept things small and we dreaded CR's a lot less).

WHERE:
any type of meeting place that can hold the invited parties. obviously its
best to keep distractions to a minimum making confrence rooms ideal, but
CR's can be held anywhere. i found attendence to my code reviews improved
when i held them in the cafeteria and advertised snacks would be provided.

don't be defensive.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: demo.zip
Type: application/x-zip-compressed
Size: 21446 bytes
Desc: not available
Url : http://mail.pm.org/archives/san-diego-pm/attachments/19990413/cfc7aacb/demo.bin
-------------- next part --------------
# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
# FUNCTION:  dPrintMonth
#
# Systematic printing of HTLM table defining a month/year pair.
#
# params
#   $_[0] = month to be printed, format: mm
#   $_[1] = year to be printed, format: yyyy
#
# returns
#   none.
# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -

sub dPrintMonth
{

    # Param is passed as mm,yyyy
    my($date_month) = $_[0];
    my($date_year) = $_[1];

    # TODO: perform some checking here

    # determine week number that starts the month
    my($tmp_week) = 1;
    my($tmp_cnt);
    my($tmp_str);

    # Next extract day/week info from date struct
    my($prim_month);
    my($prim_day);
    my($prim_year);
    my($end_month);
    my($end_day);
    my($end_year);

    my($tmp_flag) = 0;
    my($tmp_day) = 0;

    # Boldly print requested begining month
    print "<H2>$month{$date_month}, $date_year</H2>";

    # Print header info
    print "<div align=\"left\"> ";
    print "<table border=\"1\" cellpadding=\"0\" ";
    print "  cellspacing=\"0\" width=\"100%\">";
    print "  <tr>";
    print "    <td><B>Monday</B></td>";
    print "    <td><B>Tuesday</B></td>";
    print "    <td><B>Wednesday</B></td>";
    print "    <td><B>Thursday</B></td>";
    print "    <td><B>Friday</B></td>";
    print "    <td><B>Saturday</B></td>";
    print "    <td><B>Sunday</B></td>";
    print "  </tr>";

    while($tmp_week <= 52)
    {
        $tmp_date = &GetWeek("$tmp_week.$date_year");

        # Extract day values from week 
        ($prim_month, $prim_day, $prim_year, $end_month, $end_day, 
            $end_year) = split(/[\.&]/, $tmp_date);

        if(("$prim_month" eq "$date_month") ||
           ("$end_month" eq "$date_month"))
        {
            # Begin new row for every week
            print "<tr>";

            # Print each day of the week in its own cell
            $tmp_dt = "$prim_month.$prim_day.$prim_year";
            for($tmp_cnt = 0; $tmp_cnt < 7; $tmp_cnt++)
            {
                # First and last lines may contain days from other months
                ($tmp_month, $tmp_day, $tmp_year) = split(/[\.&]/, $tmp_dt);

                if("$tmp_month" eq "$date_month")
                {
                    # Build date string
                    # $tmp_str = "$short_month{$tmp_month} $tmp_day";
                    $tmp_str = "$tmp_day";
                }
                else
                {
                    # Otherwise, don't print the date
                    $tmp_str = "";
                }

                print "<td><p align=\"right\" valign=\"top\">";
                print $tmp_str, "<br>";

                # Check for any reservations
                if($tmp_str && $res{$tmp_dt})
                {
                    my($tmp_name);
                    my($tmp_email);
                    while($res{$tmp_dt})
                    {
                        ($tmp_name, $tmp_email, $res{$tmp_dt}) 
                            = split('\|', $res{$tmp_dt}, 3);

                        # Make sure '@' is transposed
                        $tmp_email =~ s/%([\da-f]{1,2})/pack(C, hex($1))/eig;
                        print "<a href=\"mailto:$tmp_email\">$tmp_name</a><br>";
                    }
                }

                print "<br><br>";
                print "</td>";

                $tmp_dt = rGetNextDay($tmp_dt);
            }

            # And close out this row
            print "</tr>";

            $tmp_flag = 1;
        }
        else
        {
            # Once found, no need to process rest of dates 
            if($tmp_flag)
            {
                goto StopProcessing;
            }
        }

        $tmp_week++;
    }

    # End this table
    StopProcessing:
    print "</table>";
    print "</div>";
    print "<br><br>";

}