SmartBear Software Webinar:  Is Agile Code Review an Oxymoron?  An Expert Roundtable

SmartBear Software Webinar: Is Agile Code Review an Oxymoron? An Expert Roundtable

or good afternoon, depending where you are. This is Esther Schindler, I want
to welcome to our webinar on “Is Agile Code Review
an Oxymoron? If you want to participate on
Twitter, we are going to use the hash tag of
#agilecodereview– all as one word– because we
don’t want to run into other strange things, like World of
Warcraft discussions or other alternatives. As I said, I’m Esther Schindler
and I’m the editor in chief and editorial director
of Software Quality Connection. I’ve been a geek for software,
anything to do with software development, for a long time. I’m really glad to have these
folks here talking about agile and code review, which
are areas that are both close to my heart. I’m going to let them do their
own introductions and then what we’re going to do is
to go into a Q&A format. The way that that I described is
this is the conference you have after the conference, in
the bar with the smart people, after two or three beers,
when you’re saying what you really think. So that’s how we’re
running it. Take it away, Jack. Introduce yourself. JACK GANSSLE: Thanks Esther. This is Jack Ganssle and I’ve
been in the embedded systems side of the computer business
since the early 1970s, so I’m a certified old fart. I am sort of the curmudgeon,
I suspect, of the group. In my industry, everything is
a little bit different than in, for example, IT or web
design or even PC programming. I am a great believer in many
of the agile practices but I’ve seen a lot of agile teams
that use agile as a cover for, basically, chaos. I’m a strong believer in code
reviews and whether they’re done with an agile
team or not, I think it’s hugely important. ESTHER SCHINDLER: OK. Next slide– to the man behind the curtain. OK. JACK GANSSLE: OK, this is
essentially my take on it. Code reviews have been
demonstrated time and time again to be probably one of
the most powerful tools we have for creating quality
software efficiently. I’m a supporter of any
development strategy that makes use of code reviews in
a disciplined fashion. ESTHER SCHINDLER: OK. Jared, you’re on. JARED RICHARDSON: All right. Hi guys, this is Jared
Richardson. I’ve, gosh, sold my first
program back in ’91, so I’ve not been in the field as long as
some of you, but I’ve been doing it for a while. The first book I wrote was Ship
It, where we actually wrote about peer code reviews
back in 2005 for the pragmatic programmers. It’s now in seven languages
and the section on code reviews is one of the most
often quoted and cited. Today I actually work for
a defense contractor. I’m an internal Agile Coach
where we do a little bit of that embedded work Jack was just
talking about, as well as some web technology. I’ve worked with Java,
Ruby, C, C++. And currently, some of
the jokes we’ve had preparing for this– I’m a backyard chicken farmer,
but we probably won’t touch on that too much in the webinar. ESTHER SCHINDLER: Although,
we did consider having #chickenfarmer as
our hash tag. JARED RICHARDSON: That
would be awesome. We could do that. No, I’m kidding. Let’s hit the next slide. My perspective on
code reviews. I remember when XP was new and
a lot of the adherents– I don’t think too many
of the leaders– were really religious about
it, where you have to be pairing all the time, 24 hours
a day, you’re going to the bathroom together. These sorts of things didn’t
really work for me at that level, but I found a lot of
benefit to the lightweight peer code review that I
think we’re going to be talking on today. I’ve been doing it for years,
love it as a practice, and it’s going to be interesting
hearing questions from the audience and what the other
panelists have to say as well. ESTHER SCHINDLER: And speaking
of other panelists– Dave. DAVE ROONEY: Hi, folks. I’m Dave Rooney, I’m an Agile
Coach and recovering software developer, although I fall
off the wagon constantly. I’ve been building software
since I had dark hair, and involved with agile since
I had some dark hair. I’m not quite as old as Jack. I haven’t been around the
industry quite that long– but longer than Jared, so
I guess I’m kind of the middle-age fart. Next slide, please. My position on this is I found
out about extreme programming in late 2000 while I was
actually pair programing. I didn’t realize
it had a term. The other person I was working
with, we just called it collaborating, but we really
were pair programming and I got into XP and the agile
world through that. When I’m working with groups,
when I’m coaching and such, I don’t specifically tell them
about pair programming. What I do say is that every line
of production code needs to be seen at least two pairs of
eyes before it’s committed. There’s a multitude of different
ways you could do that and we’ll talk about a
bunch of those today during the webinar. Thanks. ESTHER SCHINDLER: OK. So instead of all you webinar
listener-ins– instead of having everybody do
a PowerPoint and go through all their deep thoughts and
then find out how much we agree or disagree, we decided
to take on an attitude of we’re just going to open up a
question and have a panel discussion. At the end, we will open this up
to Q&A. You can enter your Qs any time you want– that
gives us some time to prioritize them and collect
them and figure out what everybody’s asking
and so forth. So our first question– as we wait– is what’s different about
code reviews when you’re doing agile? What is the distinction that you
guys see for good or ill? Let’s see– I’ll pick one randomly. Dave, you go first. DAVE ROONEY: Sure. Thanks, Esther. Well, generally it happens a lot
more often and it follows the lean principle
of just in time. At least, when I’m working with
people, I’m trying to get them to do reviews, trying to
do small reviews as quickly and as often as possible. I tell people to follow the
principle of if something hurts, do it as often
as you can. Generally, the people I deal
with, if they have been doing code reviews before,
it’s been painful. ESTHER SCHINDLER: OK. Jack? JACK GANSSLE: OK, sure. I find interesting Kent Beck’s
position on code reviews in his book Extreme Programming
Explained. He says that we know how
important code reviews are so, as he puts it, we’ll turn a knob
all the way up to 10 and we’ll be doing them
all the time. My understanding is he advocates
that mostly through the pair programming process. Might take on that is that’s
a very personal thing. Some people love
it, some don’t. I think I’d kill my peer
after about six hours, but that’s just me. But I think that that’s a very
good thing– it’s the old “two heads are better than
one” approach– if it’s being done carefully,
so that people are really concentrating and focusing. I’ve watched an awful lot of
teams operate and it’s not rare to see the non-typing
member of an XP pair with the glazed eyes, staring off into
infinity rather than really being engaged in the code. It’s like any process, it takes
discipline in order for it to be effective. My understanding is that for
some of the panelists here, their belief is that code review
should be done prior to, for example, check into the
version control system. And I presume what that means
is that’s post-testing, so you’re taking code that
mostly works. I disagree. I think there’s a couple
of magical things about code reviews. One of them, of course, is
that they find bugs. The second thing is that
testing, as it is usually practiced– and there are plenty of cases
where it’s done better, but the data is clear– testing,
as it’s usually practiced, typically only exercises about
half the code because some stuff is just really hard to
exercise, like exception handlers and stuff. What code review then does is it
ensures that there has been at least some level of auditing
about the code that both is and maybe has not been
tested, so you’re sure that it’s working. And the third thing that I think
is really magical about code review is that– and there’s tons of data
to back this up– is that it’s much more efficient
than tests, or as I prefer to call it, debugging. Code reviews find typically 70%
to 80% of the bugs, but at a much higher rate of
speed than they’re found by doing tests. So I prefer to see code reviews
done before something goes into tests. For example, using pair
programming– that’s a case where things
are done right. I guess I’d have to say it’s
something I’m pretty passionate about and I would
recommend that whatever agile method were being used, that
tests would be somewhat more decoupled than it often
is from coding. So for example, in test-driven
development, those two can be very closely coupled in order
to have it that we can interject a code review step in
there before the testing. Because let’s face
it, we’re not– we were having sort of a talk
before the start about fun and the truth is, we’re not
doing this, we’re not writing code for fun. I mean I sure, as hell hope
we’re having fun, but this is really a business endeavor and
the only reason we’re doing this is to help the business
make a profit, which means that we’ve got to use the most
efficient tools and processes that we can possibly
come up with. JARED RICHARDSON: Well, taking
the traditional prerogative of a panel to take a question and
go left with it, let me– it’s more fun this way. For what you just said, Jack,
your comments seem to imply that their tests are going
to be run once, which I would actually– or in one place– which I would disagree with. I am a huge fan of continuous
integration and having the complete test suite there,
which means unit tests, package level, integration,
everything I can get my grubby little hands on runs on the CI
server, obviously, after the code is checked in. Because what I found– and myself included– no matter how good
my intentions are or how often I– heck at this age, and I’m the
youngest of the group– as often as I remember running
all those tests, I find most developers don’t always run
the tests on their desktop before they check them in. So the process that I advocate
is run the tests that are relevant to the area
you’re working in– and we hope we get those run. I don’t count on it though. Then I get the peer code review,
then you check in the code, and then your CI server
runs everything. And for what you missed or what
your local environment didn’t have the data for or your
local environment was not matching the production
environment, we bounce it back again. How does that match with
what you were thinking? JACK GANSSLE: Well, I really
appreciate your insight there and I largely agree. Even for the most
dyed-in-the-wool advocate of the traditional plan-driven,
non-agile approaches, even the most extreme of those people
has got to take away just a pure delight in the agile
movement’s approach to tests. I mean the fact is, agile
stressing tests the way they do is just brilliant. But I don’t think that’s enough,
and I don’t think I’m contradicting anything
you would say. I think that tests– well, I would like to think
in terms of quality gates. I like that. For example, one quality
gate is the compiler. It finds syntax errors. I like to use other tools,
like lint and static analyzers and stuff. That’s another gate to
sort out problems. Inspections are another
quality gate. Test things is another one. And none of those individually
is going to be adequate, they all come together
to form a whole. The problem with tests– well, there’s a couple
of problems with problems with tests. All too often, as I said,
they miss things. It’s very hard to build a test
which is comprehensive. And the truth is, I’m a
really dumb person. All I know is my field, which is
embedded systems, so I can only speak to that. It’s very difficult in most
embedded systems to actually create an automatic test,
because someone has to watch the displays and punch the
buttons and all kinds of stuff like that. And I’ve argued with a lot of
folks about stubbing, and yes, you can stub out some
of this stuff. But, you know, as NASA says,
test what you fly and fly what you test. Don’t rely on tests on something
that’s not the flight article. Now to sort of counter my own
argument, I am seeing some companies doing some brilliant
stuff, using stuff like National Instruments’ LabVIEW. That actually has a vision
module, and I have seen tests– automatic
test setups– where they take a TV camera
that watches the embedded system, with all the blinking
lights and the displays and stuff. The vision module parses it and
can pull ASCII streams out of it, which can then run
into a test computer. And so it’s possible to do
better than most folks do with tests in the embedded space, but
it certainly is not easy. DAVE ROONEY: Jack, if
I can jump in here– it might be good to talk a bit
about definitions of test. You’re talking about literally
end-to-end tests. ESTHER SCHINDLER: You guys are
segueing into our next question anyway. So let’s put that one up on the
screen, since we’re sort of there already. Do different types of software
require different inspection strategies? DAVE ROONEY: Well, It’s not
quite what I was getting to, but what I do want to talk about
is end-to-end tests. Absolutely you need end-to-end
tests, but my experience has been– and I’ve been coaching the
last 18 months at a large telecom, writing wireless switch
software and everything back to social media and
database assurer applications for admin assistants
over the years– when you run into defects,
whether you’ve had code reviews or not, in a lot of
cases, you’re dealing with two fundamentally different
things. One is true logic programming
errors– the programmer screwed up. And the other is your
misinterpreted misunderstood, unclear requirements,
that sort of thing. But when you’re dealing with
an approach like TDD, where you are writing test after test
after test after test while you’re developing the
code, that almost eliminates 100% of, at least, the
programmer errors, and it certainly allows the people
doing the work to flesh out the cases where they’re
not understanding the requirements. You can go back, “Well, my tests
say this, so here’s why I wrote the test like this.”
It allows you to ask the questions like that. I’ve yet to see a case where a
code inspection allowed you to have a conversation like that
with someone meaningful. At least you had tests that
were, “Hey, our tests are passing, but you’re saying the
requirement’s wrong?” “I guess we missed a test case,”
if you did actually have a logic error. You have a benchmark, as opposed
to code inspections that tend to be– they’re not persistent,
as such. You’ve done the inspection, you
may or may not have found things, you may or may not
have changed things, and then it’s gone. At least tests are persistent
and can be run again. I’m talking about automated
tests, I’m not talking about just a set of manual tests and
the steps have been defined. So I think we need to
distinguish what we’re talking about here. I have difficulty, based on my
experience, reconciling with your statement that reviews are
more efficient than tests and finding bugs before the
tests, but I think that might have to do with the different
granularity or different size of the test that we’re
talking about. Yes, reviews may be more
efficient than large end-to-end tests, but when I’m
writing micro tests or unit tests I would suggest that,
no, that’s not the case- especially if I’m pairing or,
at the very least, I have somebody reviewing very close
to the time that I’m writing that test. JARED RICHARDSON:
I actually just shared a link with panelists. When I was doing some quick prep
for this conversation, the webinar, I stumbled
across a chart that– I think they actually probably
ripped it off directly from that blog posting I put out
there from Steve McConnell’s Code Complete, chapter 20. And he’s showing the various
tools we can use to write out bugs, right? The regression tests, the
code reviews, unit tests, and so forth. And the conclusion
of the chart is nothing is good enough. No one technique will
get us there, right? We have to have code reviews, we
have to have unit tests, we have to have integration
tests. The chart’s pretty
interesting. I don’t know that we want to go
into that, because we can’t really show it to
the attendees. The other thing I wanted to
throw out, and I’m very curious to hear if Jack or Dave
is familiar with the Test Driven Development for Embedded
C book that the James Grenning just released a couple
months back on the Pragmatic Programmers line. JACK GANSSLE: Yeah, actually I
reviewed that book and wrote the forward for it. James is a good buddy,
he’s a really smart guy and a good guy. We have had some spirited
debates about test driven development, shall I say? JARED RICHARDSON:
It’s more fun. JACK GANSSLE: It is. It’s a ton of fun,
it really is. This is such a fascinating
field and we can have good-natured disagreements and
enjoy the discussion and everyone can learn things
at the same time. This chart that you’ve pulled
up here, Jared, is great and basically it’s showing exactly
what I was thinking in terms of quality gates. We need to do a lot of different
things to capture all the bugs. To my knowledge, there’s not a
lot, or maybe there’s no good, real data that’s been acquired
about the efficacy, for example, of PICkit, TDD, or XP,
or whatever it might be, compared to, for example, a more
traditional approach in terms of quality and the like. And that’s unfortunate, because
it would be great to have some numbers. I know everyone feels great
about these approaches, but feeling good is not part
of the deal, you know? We want we want to have some
quantitative data. I have a ton of data on
traditional code reviews– there’s so many different
names, code inspections, you name it– that indicate that typically
you’re going to find about 80% of the bugs if you do
good inspections before you do testing. And the rate is somewhere on
the order of about 15 to 20 times faster using inspections
than tests. Now, you bring up a great
point, Dave, about what kind of tests. Listening between the lines, I
almost thought it sounded like you’re not a big advocate– I don’t want to put words in
your mouth, but it sounded like you sort of think the
inspections or the reviews are less important, perhaps,
than tests. I don’t know, that’s not
a fair thing to say. DAVE ROONEY: Well, look–
the test that you– ESTHER SCHINDLER: I’m going to
jump in here just a second. This is Esther again. I’m asking SmartBear to post
the link for that guys just shared among us, to
post that on the @SmartBear Twitter feed. So people should be able to find
that soon, if that’s not already up there. JARED RICHARDSON: And I also
posted it under the hash tag #agilecodereview as well. ESTHER SCHINDLER: Oh, cool. As long as it’s there. JARED RICHARDSON: Yeah. DAVE ROONEY: Sorry,
getting back– two quick things. One quick question to Jared–
and I’ll get back to you Jack in just a second– that reference from Code
Complete, is that the version that Steve McConnell wrote
before the XP stuff came out? JARED RICHARDSON: I don’t
know which one. It just referenced it as
being Chapter 20 and there was the chart. DAVE ROONEY: OK. OK, fair enough. Because that really was a
fundamental shift in how we approached testing
as an industry. So yes, that’s a very
interesting chart, yes, I agree with it. However, I think the world has
changed a little bit since Steve first wrote that, so I
would to take it with, well, one grain of salt, not
an entire box of it. Jack, getting back to your
comment, I absolutely agree with code inspections and I
think what might be the seed of our disagreement might be
that when I talk about tests, I’m talking about very small
tests happening on the order of a minute, two minutes that
are, if you’re taking to write, and you’re writing
code, a true TDD cycle– you may be thinking or working
from the perspective of larger tests that, yes, it does
take a lot of effort. You talk about testing as very
tough and you might only get 50% coverage from these
large tests. Absolutely. When I’m dealing with
micro tests, I can get very high coverage. Yes, I’m stubbing, mocking,
faking all over the place. So am I testing the interactions
or the integrations of things? Generally, no. I’m thinking down at the class
level, down at method level to make sure that the
bits work well. The bits still have
to go together. You still need a higher level
of testing and, yes, you do need a higher level
of inspection. I’m OK with that. I do agree that inspections will
find a lot of defects, but what is the cost of
introducing the defect and the length of time it takes to
actually find that, be it through inspection or
another mechanism? If you can get that length of
time to as near as zero as possible, be it finding a bug
through a test, finding a bug through having a pairer sitting
with you and working on it, then that’s your goal. Does that qualify as an
inspection under your definition? JACK GANSSLE: I will completely
defer to Kent Beck where he says that one of the
important reasons for pair programming is the sort of real
time code inspection and it just seems perfectly
natural for me to believe that. It’s the old “two heads are
better than one” thing. So yeah, I think it is certainly
a code review and I think that they can be,
if they’re done well, can be very effective. And the nice thing about that
is that the review is being done as the code’s being created
so you’re immediately finding problems, rather
than waiting until later in the game. I mean, you bring up an
important point, Dave, about the TDD cycle. In the embedded world there
are some TDD folks. Look at James Grenning,
for example– he coaches embedded groups. But it’s a very small part of
the market so there’s not much in terms of good data about
how effective it is. I travel all over the world
talking to embedded groups and I almost never– matter of fact, I can count them
on one hand, the number times that I’ve run into
a group that’s actually doing TDD. So I can’t really address it as
fully, perhaps, as you can, because it’s sort of the
aberration in this industry. DAVE ROONEY: By the way I had
lunch with James earlier, at noon today. He’s consulting at the client
I’m with right now and he told me to give you hell, so– JACK GANSSLE: (LAUGHING) Good. JARED RICHARDSON: Well, two
things each of you said that I would like to compare
and contrast. Dave, you made the very correct
comment that tests are persistent, right? That tests can be run over and
over and over and over, and by embedding the knowledge in the
test, you’ve got an artifact that you can keep with
the product, and I agree with that. But at the same time– Jack, you’re advocating for
the code reviews, and one thing I don’t think anyone’s
mentioned is the benefit you get out of actually improving
your people. If I’m reviewing your code and
you’re reviewing my code, I’m learning from you, Right? Yes, the tests are a persistent artifact and I love those. In fact, a lot of people think
I’m a full-time tester because I push that topic so hard. I am passionate about that. But at the same time, I’ve never
seen another technique that is good for taking a group
of junior or mid-level people and bringing them up to
the next level as pairing them up with somebody who’s as good
or better than them and doing those lightweight peer
code reviews. JACK GANSSLE: You know,
Jared, you’re spot on. I think we really have far more
agreement here among the three of us than disagreement,
and I agree about the persistency of the tests. In terms of what you’re saying
about bringing juniors up to speed, one of the powerful
things about a code review is that we get to read
great code. And whether you’re a junior
engineer or senior engineer, we can always get better. I’m always struck– this
is such a screwy field. I mean, if you want to be a
great composer, you’d listen to a lot of music, read a lot of
scores before you did any. Or a great novelist, you’d
read a lot of books. But in Computer Science 101, the
teacher says, OK, here’s a printf, now you can go home
and your homework is to write some code. It’s nuts. One of the powerful bennies, I
agree, of code review is that we are constantly reading
great code. That’s probably the best way we
can actually learn better ways of writing great code. ESTHER SCHINDLER: You guys are–
this is Esther again. I’m not sure if you guys are
aware, but Grady Booch has said on a number of occasions
that he believes that computer science courses should– in
order to major in computer science, you should have a
semester in which you are reading great code, the way
that if you’re an English major, you spend many semesters reading the classics. And he feels that it’s just as
important for developers to look at great code, whether
it’s the elegance of the original MacWrite or something,
given as an example of boy, is that just gorgeous
code, it managed to do a lot in small resources
and so forth. JACK GANSSLE: Well,
I’ll tell ya– ESTHER SCHINDLER: It also fits
into that whole notion of reading great code. JACK GANSSLE: I’ve often
advocated that computer science students shouldn’t be
allowed to write any code until junior year, because
coding is only one aspect of software engineering. I prefer to call this software
engineering over programming. Programming is a relatively
mechanical act. The harder part, by
far, is design. And Esther, like you say,
beautiful, beautiful design, those are the components
that are really tough. Programming is not the
hardest thing here. DAVE ROONEY: I think the whole
engineering debate is a webinar of its own. I’m not going to go
down that one. My only comment there is that
“beautiful” code is like saying “beautiful” music. Beautiful to whom? The three of us on the panel,
we may have completely different views of
what’s beautiful. And I certainly know that the
way you look at the world and different personalities see
things differently. I prefer loads of white
space in code when I’m reading it, so– maybe I’m ADD or something like
that, but when I have a whole screen full of just really
condensed code, it’s visual noise to me. I have very, very great
difficulty digging into that, as opposed to other people I’ve
worked with, where they want everything they can
possibly get squeezed onto the screen so they can see as much
of the code as possible, and it works for them. It’s not that they’re wrong, and
it’s not that I’m wrong– it’s we’re different. So yeah, “beautiful” code, I
think, is a realm that we should probably avoid. ESTHER SCHINDLER: We should
probably also slide up a slide or two, since we’ve managed
to go into, hopefully, interesting to other
people tangents. Have we touched on inspection
strategies as much as we want to, or shall we proceed
This is Jared. I was actually going to segue
exactly off of what he said and try to bring it
back to that. What does a beautiful
review look like? JACK GANSSLE: I’m not
sure I understand. What does a beautiful
review look like? JARED RICHARDSON: Yeah. If the review, you come away
going, man, that was awesome. You know, for me, they’re
got to be lightweight– five to 15 minutes, which means
it’s not a lot of code. It’s got to be frequent. I want the review– I’ve reviewed code five or six
times a day, but I never want it to be more than two days. If you’ve got two weeks’ or two
months’ worth of code, I call those code bombs. You’re coming in with so
much code, you’re just going to kill me. My eyes are going
to glaze over. For me, it’s not a scheduled
event on Thursday, where I bring in 30 of my coworkers, we
dim the lights, and they’re usually scheduled right
after lunch, so you can all go to sleep. I’m not going to interrupt
the way you work. I’m going to come and say,
do you have time? And if you’re busy, I’m going
to leave you alone. I’m not going to turn it into
an interrupt-driven shop. And if you say what’s this
variable mean, I’m going to go, aha! Bad variable name. If you make me explain an
algorithm to you five times, I’m going to guess it
needs to be in a subroutine with a good name. I’m going to listen
to feedback. What about you guys? Did I step on your toes
with any of those? DAVE ROONEY: No, not at all. Dave here. One of the things we’ll go
into– a later slide talking about not horror stories
but problem reviews– is where you come in with a
code bomb and people say, well, we’ve already tested it. When you suggest changes– it’s
already been tested, we don’t want to mess with it. Which defeats the
whole purpose. But if you’re doing these
scheduled reviews, then you run into that situation. That’s a real world scenario
I’ve encountered. JACK GANSSLE: Yeah, I agree. The idea of a code bomb is
just appalling to me. I think reviews– I think this is sort of
like pornography. It’s sort of hard to define,
but you know it when you see it. If you are doing more than an
hour of a review in a day, your brain is going to
be turning to mush. I often tell people that if
you’re presented with thousands of lines of code that
you have to review, you really only have two choices,
and those are either find a different job or suicide. So I think a beautiful review
is one that’s effective. I always advocate the use of
collecting metrics to see how we’re doing– OK, we did this review, what
leaked through it? And if we’re finding that a
lot of stuff is leaking through it, a lot of bugs are
leaking through it, then we should be thinking about some
sort of feedback loop to try to tune what we’re doing so that
each of these steps, each these quality gates,
is more effective. DAVE ROONEY: Right. So how about the
Zune bug, then? The famous leap-year 2008 bug
that not only was reviewed internally within Microsoft,
but it was reviewed by hundreds, if not thousands, of
people afterwards, when they posted the code. And everybody still– they looked at the code and
said, oh, well there’s the bug right there. Yes, they found part of the bug,
but it wasn’t the whole bug, and only through writing
tests would you be able to identify what the
real bug was. JACK GANSSLE: Well, number one,
they didn’t identify it. I presume they had some
level of tests. I mean, everything is tested. Even the most dysfunctional
companies I know of– and they are really broken– do testing. The real question there– would
the tester have been wise enough to have thought
about, oh, jeez, leap years? I mean, that takes sort of a
step back to thinking about this from a much broader
standpoint. And bugs will leak through. I mean, the numbers are
pretty dramatic. The absolute best-in-class
companies only find about 99% of the bugs in their code before
they ship, with very few exceptions, like some
of the avionics stuff. But 99 percent is considered
about as fast as you can get without starting to spend
gobs of money. So bugs leak out. It’s gonna happen. ESTHER SCHINDLER: OK. JARED RICHARDSON: I think our
slide has been advanced. JACK GANSSLE: Oh, right. ESTHER SCHINDLER: Have we hit
on all these various agile approaches? On TDD versus XP, et cetera? I think we’ve touched
on these things. Anybody want to add anything,
or have we– DAVE ROONEY: It’s Dave here. The question there, “Is TDD a
standalone practice or part of XP?” TDD is one that you can
absolutely do and not be doing XP at the same time. You can extract that practice. Now, XP is greater than the sum
of the parts, yadda yadda yadda, all those wonderful
mom-and-apple-pie things, but if you’re only going to do TDD,
you generally should be better than you would be
doing long delayed or long latency testing. JACK GANSSLE: Yeah, I agree. I see TDD being used standalone
or as part of XP. I’m not a fan of
TDD standalone. I think when it’s used as part
of extreme programming, I think it’s a little more
disciplined, but I will admit I have no data either way. I’ve been really struggling to
find data on the efficacy of these things and haven’t had
a whole lot of luck. JARED RICHARDSON:
This is Jared. Do you guys, when you come in
to work with a client, ever bring in single practice here
or there, like lightweight code reviews or TDD or something
like that as a way to slowly heat up the water,
to slowly effect the transformation? Or do you find it’s more
effective to bring in an entire agile approach? DAVE ROONEY: Dave here. After I try to bring in
the whole approach and they say no? Yes, then I will go with individual standalone practices. JACK GANSSLE: This is Jack. I grew up in Washington DC
during the Vietnam experience, and we were out protesting
and all this stuff. All of us hippies thought we
could change the world and, well, it turns out
we couldn’t. But we could effect
small change. I always tell companies there
are all of these things you should be thinking about doing,
but at the very least, let’s draw a line in the sand. From this point forward, at the
very least, let’s start cherry picking pieces of best
practices and using those. And then as you get comfortable
with them, we can add more and add more. ESTHER SCHINDLER: OK. Just in the effort of time– I think that this conversation
could go on for hours, I can tell. JACK GANSSLE: Yeah. ESTHER SCHINDLER: Let’s
keep moving to the next item, which is– magic. This kind is a flip side of
the “what was your great experience with code review?”
Let’s just do a quickie on the worst example of code review
you’ve seen or encountered. JACK GANSSLE: Well, Jack here. I’ve seen so many, it’s really
hard to pick a worst. There’s so many ways that
they can go wrong. The typical big problems I
see is when nobody cares. I mean, basically, people go
in and they say, oh yeah, management’s making
us do this stuff. Jeez, what a waste of time. And what happens is they don’t
put any deep thought into it, there’s no preparation, it’s
done listlessly, and then it’s just not effective. But if I could twist it– I was at the Embedded Systems
Conference a couple of weeks ago and the VP of a medical
company grabbed me and said, hey, since I started doing– they actually incorporated both
use of software standards and code reviews at
the same time. They did keep metrics. They’ve shaved 40% off
their development schedules as a result. ESTHER SCHINDLER: Any other– JARED RICHARDSON: I had a– DAVE ROONEY: Dave here. Oh, sorry. Go ahead, Jared. JARED RICHARDSON: Thanks. Jared. I was just going to share
a quick worst example. I had a person at a conference
once tell me they had a monthly, or it may have been
even more frequently than that, but it was fairly regular
code reviews where they would drag 80 people into a
room and they would pick one person for each time and they
would take everything that person had written since the
last code review and put it up on the overhead. And literally, they said, you
felt like you had been beaten with a stick. You had people that had been
in the field for 20, 30, 40 years just calling you an idiot
five ways to Friday. And, you know, he didn’t learn
anything, except to code as little as possible because then
people had less to pick on you about. ESTHER SCHINDLER: Did they
at least give you a teddy bear to hold? JARED RICHARDSON: They
didn’t tell me that. ESTHER SCHINDLER: I’m actually
not being facetious. I know that fiction writers
will, when they go around and review somebody’s work,
they will– the good ones, anyway– give somebody a teddy
bear so they have something to hold onto. It’s not a bad tip. If you’re stuck in one of those
that you can’t change, I could imagine a teddy
bear being a fairly easy thing to insert. JARED RICHARDSON: I thought
you’ve just stumbled across SmartBear’s next conference
swag giveaway. ESTHER SCHINDLER: That
wouldn’t hurt. OK, well, we do some good
questions, so I want to keep us moving so we have a few
minutes to address them. Any things that we learned
the hard way? I think we may have hit those. OK, let’s see– questions. We had some really good ones. Let’s see– one question was,
how would you rank code reviews as a quality gate
compared to unit tests, static code analysis, and
other gates? JARED RICHARDSON: Oh dear. JACK GANSSLE: I think
they often find different kinds of things. It’s sort of like saying what’s
better, breakfast, lunch, or dinner? Well, you really need
all three of those. Code reviews have a different
purpose, as Jared mentioned. Part of it is so that we
learn better ways of writing great code. I’ve seen them used
very effectively for teaching newbies. They tend to elevate the quality
of the code from the very beginning, because if I
write code that nobody’s going to look at, it’s one kind of
code– if I know my colleagues are going to go through it, I’m
a whole lot more careful. So just having the reviews
automatically elevates the code. I would call this a critical
quality gate, but it’s a different quality gate from
unit tests and static analysis, syntactical
checkers. They all are for finding
different kinds of things. DAVE ROONEY: Right, I’m in
absolute agreement with that. Now, the format of the code
review or how the code review is done, that’s where Jack and
I may quibble a little bit. And Jack, you mentioned that
there are so many ways that you can go wrong, which sounds
an awful lot like there are so many ways that people aren’t
doing pairing right. You’re saying that they aren’t
doing reviews right– well, they aren’t doing
pairing right. My experience with pair
programming has been universally positive in getting
feedback quickly. You say that if you know that
someone else is going to be looking at your code, you’re
going to do it better. That’s absolutely true and
absolutely reflects my experience over the
past 10, 11 years. If that person is sitting
beside you, engaged, and you’re swapping pairing partners
every hour or so– or even, depending on the
situation, much more often than once a week or
even once a day– then yes, you’re getting
a cause to review. People are going to be asking
you why you are doing things, so when you have to explain it
back, you’re going to do it a bit better. But in terms of the value– again, both are necessary but
insufficient on their own. You really, really need to do
both to be truly effective. JARED RICHARDSON: Yeah. This is Jared. It’s oil and tires– which one
do you need for your car? ESTHER SCHINDLER: OK. JARED RICHARDSON: You can choose
one or the other, but you make a lot better time
if you have them both. ESTHER SCHINDLER: OK,
next question. What’s your recommended flow for
performing code reviews in agile development, after
or before check-in? JARED RICHARDSON: Someone’s
trying to get this thing going past the hour mark, huh? [LAUGHTER] ESTHER SCHINDLER:
Give an elevator ride’s worth of answer. JARED RICHARDSON:
This is Jared. I tell teams you always get
a review before you check in the code. Run the tests that are specific
to what you’re doing, to your local area, then get
your review, check it in. Then we get what we get from CI,
and if you break a test in continuous integration, you’re
going to get another review. DAVE ROONEY: Dave here. I tell teams, again, that
there are many ways to actually perform
a code review. Pair programming is one,
that’s a very good one. Peer programming, where I write
the code and Jack looks at it and commits it to source
control for me, so it’s Jack’s name that’s on that code
and source control– that’s another way to do it. And there’s also the open source
model, where you have dedicated committers. You submit a patch to them, they
review it, and if they feel that it meets the various
standards and such, they’ll commit it. In all cases, the code is
reviewed prior to being committed to source control. JACK GANSSLE: And I’ll put– this is Jack– a little
different twist on it. As I said before, I think the
code review has to happen before the test, but in terms of
committing it to the source control system, I’m sort of a
believer in checking stuff in frequently, even if it’s not
ready to be checked in as people often think. In other words, there’s a lot
of reasons for a source control system and one of the important ones is the database– all of that source– is backed up every night
by the IT folks. Engineers will never back
up their own hard disks. And so even if I have incomplete
stuff, I do like to check it in and mark that as
incomplete, simply in case something horrible goes
wrong, so that the source won’t get lost. DAVE ROONEY: Jack, if I could
ask a question there– what’s your definition of frequent? So JACK GANSSLE: I would typically
say at least daily. DAVE ROONEY: OK. No, that’s fine. I tend to work on even smaller
increments than that, but– JACK GANSSLE: OK, cool. DAVE ROONEY: –daily
is a good start. JARED RICHARDSON:
This is Jared. I check in– if it’s half an hour, I’m
having a slow day. I check in all the time. But if it’s not ready
to be shared with the team, I’ll always– in any Subversion or whatever
source code you’re using– at the top level I’ll have a
folder called “the sandbox.” Anything can be checked
into the sandbox. The Sandbox user IDs, and then
put it there, and then you can move it when it’s ready
to be moved, or whatever works for you. But yeah, I agree. Check it in frequently, get
it off your computer. ESTHER SCHINDLER: OK. This is sort of a processless
scope question, which is– it was directed specifically at
Jared– how can you avoid code bombs while many
developers are developing lots of code? I think that the larger meta
question in that is, when you have bigger teams, how do you
manage to do that, to do all the right things? We know that there’s just going
to be lots of code on this project by the nature
of what needs to be done. Any thoughts on that? JARED RICHARDSON: Sure. There’s going to be lots of
code, but the problem– oh gosh, you talk about
a big question. That’ll be the next webinar. The problem is, when somebody
takes a feature– and go back to that movie,
The Money Pit, right? The plumbers kept saying
two weeks. And you come back in
a week– how much more work do you have? Two weeks, two weeks,
two weeks. You ask the standard developer
how long they’re going to be working on something,
what do they say? Well, this will take
me two weeks. Well, when you say two weeks,
that tells me you haven’t thought through it. I find it is an extremely rare
developer who can fit more than two to three to four days
worth of work in their head. So if you’re working on
something and it’s been a week, it’s because you don’t
really understand what you’re working on. You’ve glossed over
the hard parts. So to avoid a code bomb, take
whatever tasks you’ve been given, go back and break
out the old XP three by five cards, OK? Live in a database, if your
company lives in a database, but put your work on the
three by five cards. “As a role, I want features so
that–” and then on the back, “When do you know it’s done?”
Use that to break down the work into small chunks. And so, I’ve got one day’s worth
of work on this card. I’ll work on as little of one
feature or one bug, get as little work done as possible,
and then check it in. That’s what I do for myself. My desk is littered with
three by five cards. Even though I’m working on
larger projects, I take chunks of those projects and check
those in as I go. ESTHER SCHINDLER: And
presumably, review– if you have a smaller
chunk, then that’s available to be reviewed. There may be lots of chunks,
but they’re not bombs. JARED RICHARDSON: Yes. ESTHER SCHINDLER: It’s like
eating pumpkin pie at Thanksgiving– I’ll take one more
little piece. And eventually you eat
the whole pie anyway. JARED RICHARDSON: Exactly. But if anyone ever catches you,
you can say, I’m just having the one bite. That’s what you’re searching
wafer-thin mint.” JACK GANSSLE: Jack here. I have a slightly
different twist. I basically agree with you, but
I come from more of the planned, non-agile sort
of community. I find that one way to avoid
code bombs is to apply agile-like practices. For example, you may schedule
traditional, old-fashioned– even Fagan– code reviews, but frequently,
maybe every single day, you might not know what’s getting
reviewed, but it’s in the schedule so that frequently,
maybe every day, you’re doing reviews. That means that you never
accumulate a whole pile of code that you will hopelessly
try to review. ESTHER SCHINDLER: OK. We have time for, like,
two more questions. I’m looking through
our list here. This is a mentoring
question, I guess. So how can we use the metrics
to actually show that our junior programmers learn
to code cleaner and, even better, to design? In other words, hopefully both
review and agile are helping everybody develop quality
software sooner and faster, but the junior programmers, how
do you find out whether it’s working? JACK GANSSLE: Well, I think that
the worst thing we can do is use metrics generated
during the review to evaluate people. The deal with a code review
is to make great code. There are some other benefits
along the way, but as soon as we start evaluating people with
what we find, then we’re going to find people will start playing with the metrics. If the metric is very few
bugs, people will find very few bugs. If the metric is find a lot of
bugs, well, they’ll write code with a lot of bugs. I think that people should be
evaluated orthogonally from the code process development
itself. The boss may look at things
like schedule compliance, being willing to engage with
whatever processes that we’ve decided we’re going to use,
and those sorts of things. I think if we start counting
bugs and correlate that to salary, for example, boy, we’re
in for a lot of trouble. DAVE ROONEY: Dave here. To tack on that, I’m less
interested in whether a junior is, through code reviews,
becoming measurably better through some sort of metric, as
opposed to how’s the team as a whole doing? Is a team improving? Because if the team is
improving, the peer pressure within the team will bring up
people who aren’t necessarily contributing to it. Rather than looking for specific
individuals, look at the team level. JARED RICHARDSON: And
this is Jared. If you have a need to evaluate
people, don’t try to spreadsheet them. Don’t try to manage
by spreadsheet. Ask the people that are doing
the code reviews. Let people review the
people, right? If you’ve got a team, a junior
team that’s in trouble, and you pick out a handful of
seniors and say all the code reviews have to go through these
seniors for the next month, go to those senior
developers and say, how are they doing? Are they getting better or
are they getting worse? People are pretty good at
that sort of thing. ESTHER SCHINDLER: OK. This will be our
last question. Let’s see– what will be
our last question? JARED RICHARDSON: Boxers
or briefs? ESTHER SCHINDLER: Yeah. Hey, that could be good. DAVE ROONEY: Depends
on the temperature. ESTHER SCHINDLER: There was
a question about tools, especially when the pair
is not co-located. I guess we could just make this
about tools. in general. I should point out that this
webinar is sponsored by SmartBear Software, so
you should obviously check those folks out. We like to think that everything
they sell is great. But any thoughts on tools or
other code reviews/agile issues when you’re
not co-located? DAVE ROONEY: Dave here. If I can put on my extreme
programming hat, I’ve done distributed XP. We were all XP practitioners for
many years before we did this, so we weren’t learning the
process just by virtue of the company. There was myself, here in
Ottawa, somebody in Toronto, someone in Virginia, a couple
people in the Bay Area, one guy in Seattle– we used Skype and VNC. We paired virtually everything
on the system and it worked surprising well. It surprised me how well it
worked, mainly because the people were dedicated software
craftsmen and were willing to pair. We were willing to write tests
and we’re constantly reviewing the code that we’re writing. JACK GANSSLE: OK, Jack here. I’ve seen some pretty
spectacular failures because of this, because of geographic
problems. But even worse, the
temporally-dispersed teams, where you’re 12 hours away from
someone else, so what happens is– it can still
be very effective. I mean, it’s still possible. CodeCollaborator, for example,
I’ve seen used with teams scattered around the
world very well. But it will always be less
efficient than having people that are at least, more or less,
in the same time zone, because half the team is asleep
half the time and you just can’t get their
attention. I think ultimately, if you take
all the discussion we’ve had in the last hour or so, most
of this is going to come down to the true professionalism
of the members of the team. The teams I see with the biggest
failures are the ones who just do not act
professionally, and those who are truly devoted to the
profession and behave, you know, engaged in the highest
possible way, tend to make things work no matter what these
sorts of challenges are. ESTHER SCHINDLER: OK. JARED RICHARDSON: Just
one last line. This is Jared. When people are distributed,
Andy Hunt always likes to say that the bandwidth
in email is low. I don’t know if that quote
originated with him, but I’ve heard him say it enough,
I just sort of stick his name on it now. When you’re trying
to figure out if somebody is mad at you– I mean, think of how many times
we’ve had arguments over email, right? Somebody’s emailed something,
you get mad, and it just escalates. Don’t use any tool as your
last line of defense. Get a video camera
up– especially if someone’s remote– to discuss what came
out of the tool. If you’re using
CodeCollaborator, start there, but don’t stop there. Use it as a way to engage the
other team members in conversations. That’s the best way to move past
the cold, gray ASCII of the screen and actually
engage your teammate. ESTHER SCHINDLER: OK. This looks like a good
place to end on. Thank you. I want to say thank you very
much to all of our panelists who– as I said, I think this
could be at least two hours worth of beer in this, our
virtual barroom here at the conference. We will be coming back to these topics in upcoming webinars. I’m glad that just about all
of you hung out with us the whole time. I think there’s plenty of stuff
to talk about more. I encourage you to, obviously,
check out CodeCollaborator. Also, to check out Software
Quality Connection. We will do our best to let you
know about a recording of the webinar, as well as follow ups
for the questions that we didn’t get to, because
they were many. Thank you very, very much
for participating and have a good day. JACK GANSSLE: Thanks folks,
it was a lot of fun. DAVE ROONEY: Thank you.

Leave a Reply

Your email address will not be published. Required fields are marked *