Re: Will you help me put together a code review checklist?

Discussion in 'C++' started by Francis Glassborow, Nov 15, 2010.

  1. On 14/11/2010 12:49, James Kanze wrote:
    > On Nov 13, 2:49 am, Andrew<> wrote:
    >> On Nov 12, 3:53 pm, DeMarcus<> wrote:

    >
    >>> I've read a lot of good books regarding writing safe code,
    >>> as for instance Scott Meyer's Effective C++ and Sutter&
    >>> Alexandrescu's C++ Coding Standards.

    >
    >>> However, what I realized the other day is that what we need
    >>> is a condensed checklist

    >
    >> Years ago I tried to create these at various places. I spent
    >> ages distilling all the good rules from good book together in
    >> long and short forms and even managed to get them established
    >> as coding stds in some places. I now think this was a waste of
    >> time. Let me explain why:

    >
    >> o Code review is a good thing, so we should do more of it. I
    >> reckon pair programming achieves the ideal, where it is being done
    >> *all* the time.

    >
    > No. Pair programming does not address the issues that code
    > review addresses. Good code review requires someone from
    > outside to review the code, not someone who is intimely involved
    > in it. Globally considered, pair programming is not cost
    > efficient (but it can be useful in specific cases, e.g. bringing
    > a new hire up to speed in your code).


    I think that is too strong an assertion. Pairing two gifted programmers would
    normally be inefficient but I can imagine pairings of average programmers that
    would prove more efficient than having the two work independently.

    The other issue is that pair programming was designed to work in situations
    where it is highly desirable for all members of the team to be knowledgeable
    about the projects entire code base. Note that pairings are not intended static
    nor should they be if this objective is to be achieved.



    --
    [ See http://www.gotw.ca/resources/clcm.htm for info about ]
    [ comp.lang.c++.moderated. First time posters: Do this! ]
     
    Francis Glassborow, Nov 15, 2010
    #1
    1. Advertising

  2. Francis Glassborow

    James Kanze Guest

    On Nov 15, 12:25 am, Francis Glassborow
    <> wrote:
    > On 14/11/2010 12:49, James Kanze wrote:
    > > On Nov 13, 2:49 am, Andrew<> wrote:
    > >> On Nov 12, 3:53 pm, DeMarcus<> wrote:


    > >>> I've read a lot of good books regarding writing safe code,
    > >>> as for instance Scott Meyer's Effective C++ and Sutter&
    > >>> Alexandrescu's C++ Coding Standards.


    > >>> However, what I realized the other day is that what we need
    > >>> is a condensed checklist


    > >> Years ago I tried to create these at various places. I spent
    > >> ages distilling all the good rules from good book together in
    > >> long and short forms and even managed to get them established
    > >> as coding stds in some places. I now think this was a waste of
    > >> time. Let me explain why:


    > >> o Code review is a good thing, so we should do more of it. I
    > >> reckon pair programming achieves the ideal, where it is being done
    > >> *all* the time.


    > > No. Pair programming does not address the issues that code
    > > review addresses. Good code review requires someone from
    > > outside to review the code, not someone who is intimely involved
    > > in it. Globally considered, pair programming is not cost
    > > efficient (but it can be useful in specific cases, e.g. bringing
    > > a new hire up to speed in your code).


    > I think that is too strong an assertion. Pairing two gifted
    > programmers would normally be inefficient but I can imagine
    > pairings of average programmers that would prove more
    > efficient than having the two work independently.


    Pairing two programmers will always result in better code,
    faster. If those are the only goals, however, it's almost never
    the most cost effective means of achieving this end. In the
    end, everything depends. If you've got programmers doing
    nothing, and for whatever reason can't or don't want to make
    them redundant, then pairing programmers might be free. In
    which case, go for it.

    Just don't expect pairing programmers to eliminate the need of
    a good code review, from someone not involved in the code.

    > The other issue is that pair programming was designed to work
    > in situations where it is highly desirable for all members of
    > the team to be knowledgeable about the projects entire code
    > base. Note that pairings are not intended static nor should
    > they be if this objective is to be achieved.


    Almost all of the projects I've seen are large enough that no
    one person could possibly keep everything in their head at one
    time. The correct answer here is proper documentation, so all
    members of the team can quickly find the information they need,
    when they need it.

    --
    James Kanze


    --
    [ See http://www.gotw.ca/resources/clcm.htm for info about ]
    [ comp.lang.c++.moderated. First time posters: Do this! ]
     
    James Kanze, Nov 15, 2010
    #2
    1. Advertising

  3. Francis Glassborow

    James Kanze Guest

    On Nov 21, 8:37 am, Mark P <> wrote:
    > On Nov 15, 3:16 pm, James Kanze <> wrote:


    >> On Nov 15, 12:25 am, Francis Glassborow


    >> Almost all of the projects I've seen are large enough that no
    >> one person could possibly keep everything in their head at one
    >> time. The correct answer here is proper documentation, so all
    >> members of the team can quickly find the information they need,
    >> when they need it.


    > In my experience, the ability to execute a reviewers unit test and
    > (perhaps) compare results provides a lot of value added.


    I'm not sure what you're arguing for. That reviewers should
    write unit tests? Instead of or in addition to the original
    programmer? My experience has been that the best results come
    from having the unit tests as part of the code being reviewing.
    In particular, a code unit can fail review because its unit
    tests aren't exhaustive enough.

    > My company has a limit on the size of the word product but we
    > often miss things and I think we have 'proper documentation'.


    I'm not sure what you mean by "the word product". But part of
    the motivation of code review is to ensure that a programmer
    reading the code and the available documentation can understand
    the code without problems. If he can't, it fails the review.

    --
    James Kanze


    --
    [ See http://www.gotw.ca/resources/clcm.htm for info about ]
    [ comp.lang.c++.moderated. First time posters: Do this! ]
     
    James Kanze, Nov 25, 2010
    #3
  4. On Nov 14, 7:25 pm, Francis Glassborow
    <> wrote:
    > On 14/11/2010 12:49, James Kanze wrote:
    >
    >> On Nov 13, 2:49 am, Andrew<> wrote:
    >>> On Nov 12, 3:53 pm, DeMarcus<> wrote:

    >
    >>>> I've read a lot of good books regarding writing safe code,
    >>>> as for instance Scott Meyer's Effective C++ and Sutter&
    >>>> Alexandrescu's C++ Coding Standards.

    >
    >>>> However, what I realized the other day is that what we need
    >>>> is a condensed checklist

    >
    >>> Years ago I tried to create these at various places. I spent
    >>> ages distilling all the good rules from good book together in
    >>> long and short forms and even managed to get them established
    >>> as coding stds in some places. I now think this was a waste of
    >>> time. Let me explain why:

    >
    >>> o Code review is a good thing, so we should do more of it. I
    >>> reckon pair programming achieves the ideal, where it is being done
    >>> *all* the time.

    >
    >> No. Pair programming does not address the issues that code
    >> review addresses. Good code review requires someone from
    >> outside to review the code, not someone who is intimely involved
    >> in it. Globally considered, pair programming is not cost
    >> efficient (but it can be useful in specific cases, e.g. bringing
    >> a new hire up to speed in your code).

    >
    > I think that is too strong an assertion. Pairing two gifted programmers would
    > normally be inefficient but I can imagine pairings of average programmers that
    > would prove more efficient than having the two work independently.


    I think it's too weak a reponse. When two gifted programmers paired in
    that context they will be so annoyed that either one of them would
    have done a better work. The most important part of programming work
    is concentration (others call it "flow") when mental images are
    translated into code. Any distraction, especially another programmer
    looking over the shoulder would destroy it. The principle of high
    quality programming is to ensure the least fragmentation of the code
    base between programmers, with a single person fully responsible for
    doing a logicaly complete piece of work being optimal. Collaborating
    -- yes, sharing -- no. I think Brooks was writing about those issues
    some 30 years ago.
    Of course, when it comes to novices and/or doing all sorts of trivial
    stuff, almost anything can be become an improvement. The logical error
    is to translate it to "programming proper," i.e. when serious
    programming tasks are solved by experts.

    > The other issue is that pair programming was designed to work in situations
    > where it is highly desirable for all members of the team to be knowledgeable
    > about the projects entire code base. Note that pairings are not intended static
    > nor should they be if this objective is to be achieved.


    There are better venues to educate about code base than pairing
    programmers. Shared high quality documentation is one such way.

    I frankly can't see any value in pairing of expert programmers under
    any circumstances.


    --
    [ See http://www.gotw.ca/resources/clcm.htm for info about ]
    [ comp.lang.c++.moderated. First time posters: Do this! ]
     
    Gene Bushuyev, Nov 25, 2010
    #4
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. sean

    Openmore checklist

    sean, Jan 7, 2004, in forum: VHDL
    Replies:
    2
    Views:
    943
  2. roni
    Replies:
    2
    Views:
    391
    Eliyahu Goldin
    Jul 17, 2005
  3. www
    Replies:
    51
    Views:
    1,501
  4. Ian Collins
    Replies:
    0
    Views:
    394
    Ian Collins
    Nov 16, 2010
  5. Martin Hansen
    Replies:
    0
    Views:
    119
    Martin Hansen
    May 6, 2010
Loading...

Share This Page