Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > comp.arch.embedded > #18220

Re: D'oh! Oh, for lack of a code review.

Show all headers | View raw

On 9/21/2014 8:51 PM, "Bruce Varley" wrote:

> Good topic for airing from a number of angles. I'm part of a very
> small team developing specialised code for large manufacturing plants,
> using arcane languages and platforms that mean that an 'ordinary'
> programmer isn't going to be of much help.

You'd be surprised at how many parallels pertain to other industries,
regardless of languages, platforms, etc.

I.e., application domains tend to be fairly specialized (unless you're
writing desktop code -- though even there!).  A person developing
motor control code and another developing software to analyze blood
samples have very little common ground (besides, perhaps, a language).

> I've made the point to our
> team leaders that in the wider embedded software industry, code
> overview/reviewing is a strongly recommended practice, but for the
> usual reasons, getting anyone assigned to the job never happens.

IME, you can't treat it as a checkoff item that happens at a
specific place(s) in the development process.  The results are
much better if your colleagues are intimately aware of the
project, have discussed aspects of it with you in the past, etc.

At the very least, they feel less like "outsiders" and more
"invested" in the (your) project.

One place I worked had token rewards (trivial things -- a candy bar!)
to acknowledge value contributed by others to "your" project.  Sounds
silly but it makes the event more memorable to all parties.

> The few times I've managed to get a colleague, unofficially, to do a
> review for me, the results haven't been great.  It's relatively easy
> to catch the superficial stuff, such as pointless looping and crappy
> naming. But the things that cause trouble are embedded much deeper,
> and catching them would require a thorough understanding of the
> situation - a programmer-to-programmer brain transplant almost - to
> achieve much.

Exactly.  Doing this after the fact is like proofreading a novel:
yeah, you might catch some spelling, punctuation, grammatical
errors -- but, any comments you have on the storyline are too late
to be effective (then why bother offering them?).

OTOH, if you'd been discussing your "plot line" with that future
proofreader all along, you might have been able to benefit from
these observations early on.

> I'd be interested in how this is dealt with by others.
> How counterproductive is lack of extreme detail in the functional
> specification?

I find a detailed specification makes things a LOT easier to
critique.  I now write user manuals early on as these let
folks (clients, potential users, myself, etc.) envision REAL
situations and how they will be handled.  It's amazing
how many big holes you can find in a client's proposal once
you start rewriting it in this form ("gee, we have a way of
CREATING foobars but no way of DELETING them!  Oh, and, by
the way, how many of these can we create?  And, how does this
affect the usability of the system?")

> Is there such a thing as a methodology that people
> would recommend?

I spend a lot of time documenting exceptions and special cases
in my source commentary.  "I'm doing it this way BECAUSE..."

In recent projects, I am removing this from the codebase and,
instead, preparing "tutorials" -- essentially, organizing my
working notes in a manner that allows others to see how I
came to a particular decision (instead of just documenting
the decision itself).

This also helps identify the cases that *need* to be verified
when the code is tested.  And, lets others examine the test
suite to consider other situations that don't appear, there.

> Is top-down or bottom up better (and is this
> question meaningful?). Is it possible to estimate review time per
> line, or on some other basis?

I've never seen any productive results doing "line by line" style
reviews.  People QUICKLY "zone-out".  It's too tedious and yields
too few "hits".  Rather, indicate WHAT you are trying to do and HOW
you are doing it.  Let your reviewers ASSUME you have implemented
this correctly (why examine every line of code?).  Indicate how
you have tested it (did you forget some boundary condition?).
Then, let them think about what COULD go wrong with your overall
approach -- and YOU explain why that's NOT a problem...

"What happens if we want to create 5,000 foobars?  How would a user
manage all of those 'objects'/names?"

Back to comp.arch.embedded | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

D'oh!  Oh, for lack of a code review. Tim Wescott <seemywebsite@myfooter.really> - 2014-09-21 16:23 -0500
  Re: D'oh!  Oh, for lack of a code review. Clifford Heath <no.spam@please.net> - 2014-09-22 09:02 +1000
    Re: D'oh!  Oh, for lack of a code review. Simon Clubley <clubley@remove_me.eisner.decus.org-Earth.UFP> - 2014-09-22 00:21 +0000
      Re: D'oh!  Oh, for lack of a code review. Clifford Heath <no.spam@please.net> - 2014-09-22 10:58 +1000
        Re: D'oh! Oh, for lack of a code review. Les Cargill <lcargill99@comcast.com> - 2014-09-21 22:28 -0500
      Re: D'oh!  Oh, for lack of a code review. rickman <gnuarm@gmail.com> - 2014-09-21 22:16 -0400
    Re: D'oh!  Oh, for lack of a code review. Paul E Bennett <Paul_E.Bennett@topmail.co.uk> - 2014-09-22 01:28 +0100
      Re: D'oh!  Oh, for lack of a code review. Don Y <this@is.not.me.com> - 2014-09-21 20:16 -0700
        Re: D'oh!  Oh, for lack of a code review. Bruce Varley - 2014-09-22 11:51 +0800
          Re: D'oh!  Oh, for lack of a code review. Don Y <this@is.not.me.com> - 2014-09-21 21:19 -0700
        Re: D'oh!  Oh, for lack of a code review. Paul E Bennett <Paul_E.Bennett@topmail.co.uk> - 2014-09-22 22:20 +0100
          Re: D'oh!  Oh, for lack of a code review. Don Y <this@is.not.me.com> - 2014-09-22 16:28 -0700
            Re: D'oh!  Oh, for lack of a code review. rickman <gnuarm@gmail.com> - 2014-09-23 00:43 -0400
        Re: D'oh!  Oh, for lack of a code review. rickman <gnuarm@gmail.com> - 2014-09-22 18:48 -0400
          Re: D'oh!  Oh, for lack of a code review. Don Y <this@is.not.me.com> - 2014-09-22 15:59 -0700
            Re: D'oh!  Oh, for lack of a code review. rickman <gnuarm@gmail.com> - 2014-09-23 00:49 -0400
    Re: D'oh!  Oh, for lack of a code review. Don Y <this@is.not.me.com> - 2014-09-22 16:15 -0700
      Re: D'oh!  Oh, for lack of a code review. Don Y <this@is.not.me.com> - 2014-09-22 16:30 -0700
  Re: D'oh!  Oh, for lack of a code review. David Brown <david.brown@hesbynett.no> - 2014-09-22 10:22 +0200
  Re: D'oh!  Oh, for lack of a code review. Hans-Bernhard Bröker <HBBroeker@t-online.de> - 2014-09-22 19:08 +0200
    Re: D'oh!  Oh, for lack of a code review. Tim Wescott <seemywebsite@myfooter.really> - 2014-09-22 13:59 -0500
      Re: D'oh!  Oh, for lack of a code review. Paul Rubin <no.email@nospam.invalid> - 2014-09-22 12:18 -0700
        Re: D'oh!  Oh, for lack of a code review. tim <tim@seemywebsite.com> - 2014-09-23 00:27 -0500
          Re: D'oh!  Oh, for lack of a code review. Robert Wessel <robertwessel2@yahoo.com> - 2014-09-23 18:35 -0500
      Re: D'oh!  Oh, for lack of a code review. David Brown <david.brown@hesbynett.no> - 2014-09-22 21:26 +0200
  Re: D'oh!  Oh, for lack of a code review. John Speth <johnspeth@yahoo.com> - 2014-09-22 16:24 -0700
    Re: D'oh!  Oh, for lack of a code review. Don Y <this@is.not.me.com> - 2014-09-22 16:34 -0700
    Re: D'oh!  Oh, for lack of a code review. rickman <gnuarm@gmail.com> - 2014-09-23 01:11 -0400

csiph-web