> If you give actionable (reasonably emotion free) feedback in the beta
For me anyway, the emotional content of feedback can have meaning. I can
think of a specific piece of code I reviewed years ago that used 4k bytes of
stack space in a function that was called at DPC level. My feedback during a
code review was something like “I can’t prove this will ever crash, but my
gut feeling is using 4K of stack at DPC level is risky and feel concerned
about this code”. About a month later, a bug was found where during a
network packet indicate, a callback into a packet send routine happened,
consuming a large amount kernel stack, and this function was called, causing
a crash. Even though my emotions tend to be a somewhat noisy source of
information, lots of times they have given me extremely valuable insights
into software.
Perhaps if someone wants to give feedback that feels emotional, the emotion
is just a piece of information that some complex interaction may happen in
the code, and giving some specific examples that backs up the emotion would
be useful feedback.
An example, instead of “This code sucks”, feedback like “This code reminds
me of code that failed because of X, Y, and Z. I can’t prove it will fail
due to X, Y, and Z, but feel it needs more analysis to assure it’s ok”. I
wish every bug were just simple black and white, but unfortunately some are
more shades of grey. I know when reviewing code, I’ve often found bugs when
I’m able to shift my mindset, and consider the possibility that something
isn’t actually like I thought it was. Processors have this nasty habit of
doing exactly what they are told to do, not what our intent was. Every time
there is a buffer overrun, the programmer’s thinking was it could never
happen. Any process that makes your brain see reality more clearly will
likely improve the stability of code.
As a side note, some computer languages also have less ambiguity about
programmer intent vs. actual action. A quick example, if I write “a[12] =
6;” in C, there is absolutely no guarantee I didn’t just corrupt the memory
just after a, and an hour later that corrupted memory causes a crash. If I
write that same statement in say C#, I don’t believe there is any chance I
just corrupted memory, although I may have just received a compile or
run-time error. In C++, I could teach the compiler that a is something with
bounds, and it should generate code to check that a subscript access doesn’t
violate those bounds (it won’t be automatically guaranteed like in C#).
On the flip side, if you receive a bug report that says “This code sucks”,
it might be useful to ask, what about the code is causing that emotion. It’s
NOT a criticism of you personally, it’s more an inability of the
reviewer/user to articulate in clear words what the problem is, although
they know something seems wrong. I’m sure people doing user interface design
almost always get feedback of “This sucks” and need to interact with the
feedback source to understand the hidden meaning of “This sucks”, and break
it down into little actionable pieces. UI science has extensive tools to
actually figure out how to translate “it sucks” into actionable specific
understanding.
Explaining your personal experience that gives you the emotional response of
“This sucks” also can be useful. For example, if I just have ported 20,000
lines of Linux C code to Windows, and the first time I try to compile it get
an OACR message that my code has 6456 errors, my reaction often would be
“OACR sucks, big-time”. My response may be to then turn off OACR for a long
time, because it’s clearly not helping. Knowing my experience, and my
context of when I react with “OACR sucks” would help the OACR developers
understand that a bit more accurate description might be “If OACR finds
errors in 25% of my code, perhaps the developer knows already, and perhaps
OACR should shut up for a while, or perhaps give a single message like Dear
developer, I see you’re a bit overwhelmed with your new code, so we will
suppress OACR feedback until the error ratio drops below 3%”.
I personally think having automated code checking facilities is a terrific
thing. If I can feel more secure about the lock correctness of some code by
adding “__guarded_by” annotations, that’s a good thing. An important issue
is: can I trust my effort of adding annotations will be justified in better
code. That’s where the “feel more secure” part comes from. If I trust
prefast to improve things in exchange for my work, I’m signed up. To gain
trust in prefast’s ability to check lock correctness (and every other kind
of correctness) some sample cases I can look at and run would be very
helpful. Trust has to be earned, and is not just always assumed (I guess
that’s why I’m a computer scientist and not a priest). Finding errors in MY
code is one way prefast earns my trust, but having it’s abilities and
weaknesses demonstrated in samples would also improve my trust. So, dear
Microsoft prefast team, I would trust prefast more if you could supply a
bunch of samples that demonstrate errors it can find (that the compiler
can’t), like under “tools\education\prefast samples”. It would also help to
have info on what kinds of errors it can’t find, so I know where to focus my
effort during code reviews.
Just an opinion…
Jan
__________ Information from ESET NOD32 Antivirus, version of virus signature
database 4367 (20090825) __________
The message was checked by ESET NOD32 Antivirus.
http://www.eset.com