Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > C++ > Re: Will you help me put together a code review checklist?

Reply
Thread Tools

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

 
 
Francis Glassborow
Guest
Posts: n/a
 
      11-15-2010
On 14/11/2010 12:49, James Kanze wrote:
> On Nov 13, 2:49 am, Andrew<marlow.and...@googlemail.com> wrote:
>> On Nov 12, 3:53 pm, DeMarcus<use_my_alias_h...@hotmail.com> 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! ]

 
Reply With Quote
 
 
 
 
James Kanze
Guest
Posts: n/a
 
      11-15-2010
On Nov 15, 12:25 am, Francis Glassborow
<francis.glassbo...@btinternet.com> wrote:
> On 14/11/2010 12:49, James Kanze wrote:
> > On Nov 13, 2:49 am, Andrew<marlow.and...@googlemail.com> wrote:
> >> On Nov 12, 3:53 pm, DeMarcus<use_my_alias_h...@hotmail.com> 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! ]

 
Reply With Quote
 
 
 
 
James Kanze
Guest
Posts: n/a
 
      11-25-2010

On Nov 21, 8:37 am, Mark P <email.powe...@gmail.com> wrote:
> On Nov 15, 3:16 pm, James Kanze <james.ka...@gmail.com> 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! ]

 
Reply With Quote
 
Gene Bushuyev
Guest
Posts: n/a
 
      11-25-2010

On Nov 14, 7:25 pm, Francis Glassborow
<francis.glassbo...@btinternet.com> wrote:
> On 14/11/2010 12:49, James Kanze wrote:
>
>> On Nov 13, 2:49 am, Andrew<marlow.and...@googlemail.com> wrote:
>>> On Nov 12, 3:53 pm, DeMarcus<use_my_alias_h...@hotmail.com> 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! ]

 
Reply With Quote
 
 
 
Reply

Thread Tools

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
Re: Will you help me put together a code review checklist? Ian Collins C++ 0 11-16-2010 01:37 PM
What is code review? (Java code review) www Java 51 05-15-2007 01:10 PM
if 'ejecting' a cd is how you remove it, then when you put it in, you must be injecting a cd. Doc Martian Computer Information 4 06-09-2006 11:04 PM
WYSIWYG content management system; how to put together and make it work anoniem Java 3 09-14-2004 12:58 PM
how do I put sound and photo together GLENYS ELLWOOD Computer Support 1 10-10-2003 07:35 PM



Advertisments
 



1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57