What is code review? (Java code review)

[Chris Smith]

I would be embarrassed, and try to modify my habits to not do the
error again, but I would not blame the person who found it.

I find being embarrassed about programming errors to be nearly as
unhealthy a habit as being hostile, actually. In a healthy development
environment, programmers realize and accept that errors occur;
understand that they are normal, fix them, and move on. Change habits,
sure if it's reasonable. It's often not, and adopting techniques to
eradicate all possible programming errors is likely to lead to nothing
at all getting done. Embarrassment should never be appropriate.

(One qualifier: the kind of embarrassment that leads to having a good
laugh with your coworkers might be an exception.)

 

[Daniel Pitts]

I find being embarrassed about programming errors to be nearly as
unhealthy a habit as being hostile, actually. In a healthy development
environment, programmers realize and accept that errors occur;
understand that they are normal, fix them, and move on. Change habits,
sure if it's reasonable. It's often not, and adopting techniques to
eradicate all possible programming errors is likely to lead to nothing
at all getting done. Embarrassment should never be appropriate.

(One qualifier: the kind of embarrassment that leads to having a good
laugh with your coworkers might be an exception.)

I agree. I actually deployed some code live recently that was way
broken. It had gone through our QA process, but they managed to miss
a chunk of functionality not working. I would have been much happier
to have someone tell me that the code was broken, rather than have a
P! bug on my queue.

 

[chandan]

I've heard of teams that do group code reviews, and have the
whole team in a room with a projector going over every line.
That seems insane to me.

Insane to do if someone's life depends on the software?

»[This] is one of just four outfits in the world to win
the coveted Level 5 ranking of the federal governments
Software Engineering Institute

Consider these stats : the last three versions of the
program -- each 420,000 lines long-had just one error
each. (...)

Otherwise, the hour-long meeting is sober and revealing, a
brief window on the culture. For one thing, 12 of the 22
people in the room are women, many of them senior managers
or senior technical staff. [This] group, with its
stability and professionalism, seems particularly
appealing to women programmers.

The central group breaks down into two key teams: the
coders - the people who sit and write code -- and the
verifiers -- the people who try to find flaws in the code.

The two outfits report to separate bosses and function
under opposing marching orders. The development group is
supposed to deliver completely error-free code, so perfect
that the testers find no flaws at all. The testing group
is supposed to pummel away at the code with (...)
scenarios and simulations that reveal as many flaws as
possible. (...)

The developers have even begun their own formal
inspections of the code in carefully moderated sessions, a
rigorous proof-reading they hope will confound the
testers. [The] group now finds 85% of its errors before
formal testing begins, and 99.9% before the program is
delivered (...)«

http://www.fastcompany.com/online/06/writestuff_Printer_Friendly.html

Code review is not a process to find out the bugs/logical errors/
function malfunctioning behaviour. It is just a process where
experienced programmer/s have a look at your workspace and tries to
point your attention towards some general mistakes which a new comer
could do, to name a few...
1. Giving variable/methods/class/package names which make no sense for
others , like var1,functionOne() et all.
2. Placing constant strings to different class.
3. If you are writting a DAO , then you may put your queries in the
same class.
4. Putting much logic in the same method, which could have been
segregated.
5. Stretching the length of a class to its limit.

So the code review is the process in which best practices are advised
to you so that you start following them, as it makes one's life easier
when re-visited your own code after some decent gap.

And above all writting a logic is not the end of road, it also matters
sometimes how others can easily grasp your code.

Chandan

 

[Joe Attardi]

Code review is not a process to find out the bugs/logical errors/
function malfunctioning behaviour. It is just a process where
experienced programmer/s have a look at your workspace and tries to
point your attention towards some general mistakes which a new comer
could do

I have to disagree. Experienced programmers make mistakes just like
anybody else. And it isn't just for mistakes, it certainly is also for
finding potential bugs/logical errors/etc. This is not unique to
programming. It's why professional journal articles are peer reviewed
before they are published. You will always miss some problems in your code.

 

[Brian Raven]

A decade or so ago there was a movement called "egoless
programming," which encouraged programmers to write and fix
and improve "the" code rather than "my" code. I never worked
at a shop where this notion was enshrined in formal procedure,
but the best programmers I worked with all shared the notion
that the code was "ours" and not "mine."

I think it's a healthy attitude, and a differentiator
between professionals and hobbyists.

I recall first reading about egoless programming in Weinberg's
Psychology of Computer programming which, according to a quick google,
was first published in 1971. I agree its a healthy attitude. A key
point to remember with code/design reviews, I find, is to not take it
personally, whether you are reviewer or reviewee.

 

[Richard Reynolds]

Hmm... I agree. I didn't catched the metaphor.

--harald

Thanks Lew, indeed I was, different languages I guess.

Lew said:

And it doesn't look like Tony Blair is going to be able to hold his seat
much longer. Something about the British public blaming him for involving
the UK in George Bush's Middle East fiasco of oppression and hegemony.

Hmm, who is the President of the United States:

( ) George W. "Dubya" Bush
( ) Adolf Hitler
?

looks like it's Brown pretty shortly, 14th June I think I heard on the radio
today, and as for the U.S., at least you guys have a rule that'll stop that
muppet serving another term ... if Blair had've gone after term two he'd
have been remembered as a great Prime Minister!

 

[tjmadden1128]

I find being embarrassed about programming errors to be nearly as
unhealthy a habit as being hostile, actually. In a healthy development
environment, programmers realize and accept that errors occur;
understand that they are normal, fix them, and move on. Change habits,
sure if it's reasonable. It's often not, and adopting techniques to
eradicate all possible programming errors is likely to lead to nothing
at all getting done. Embarrassment should never be appropriate.

(One qualifier: the kind of embarrassment that leads to having a good
laugh with your coworkers might be an exception.)

You might be right. I have been bombarded over the years with zero
tolerance for software defects and the fact that there is a process to
achieve it by management that I have a hard time with bugs in the code
I wrote. Logically, I know errors occur, but when you are told they
cost a company X thousands of dollars, time spent, and client
confidence, you want to try harder to avoid making them. The endless
parade of TQM methods such as Six Sigma, RUP, XP, and others convinces
you that it is possible. It's scary that I might have bought into all
of that.

Tim

 

[Andy Dingley]

Boss: I think we should use an SQLServer.
Employee: What colour ?
Boss: Blue - they have most RAM.

Look up "pointy haired boss" on Wikipedia. This joke is more subtle
than you realise.

 

[Andy Dingley]

Nobody likes his code to be found has "error". It is completely OK for
the systems to be found behave incorrectly one or two years later,
because it is different branch people (or even different company) to
debug and maintain the software.

I guess your code is always perfect anyway. After all, you must have
at least 20 years experience to have such an antiquated attitude to
software development. Are you by any chance a MUMPS coder?

Go and find something to read. Mc Connell's "Rapid Application
Development" might do for starters (it's only 10 years out of date, so
it'll lead you into the present gradually). Otherwise something by
Martin Fowler or Kent Beck with "Agile" or "Refactoring" in the title.
Even just bwosing wikipedia for "ego-less coding", "collective code
ownership" or "Getting a first resemblance of a clue, 101"

 

[Lew]

The endless
parade of TQM methods such as Six Sigma, RUP, XP, and others convinces
you that it is possible. It's scary that I might have bought into all
of that.

Perhaps the "parade" convinces you, but it convinces me of the opposite.

There is a little booklet called /Wicked Problems, Righteous Solutions/ that
is worth a read. It points out that software development is a "wicked"
problem in the formal sense - its solutions are highly sensitive to initial
analysis in generally unpredictable ways. If the problem were so easily
solvable then one or another of these products would have already captured the
market.

The endless parade of cosmetic products alleged to reverse the appearance of
fine lines caused by aging does little to convince me that a Fountain of Youth
is possible, either. Generally, endless parades of products that promise the
impossible convince me only that P. T. Barnum was right in his assessment of
the public.

This is not to say that you should give up and just accept bugs. Au contraire.

 

[Karl Uppiano]

Anyway, I'm always for light process. And that is what right now I'm
heavily relying on 'pair and montior'. By pairing developers, you have
a higher chance of having better documented code. We then have one of
two team members who's sole responsibility is to monitor everything
that goes into CVS and flag anything that disagree with our
development handbook. So far this is a low impact approach that seem
to be working.

One of the most productive projects I ever worked on was one in which the
developers swapped coding and JUnit tasks. You have to understand the code
pretty well to write the JUnits. And the second set of eyes gave us a
tremendous insight into problems, optimizations, clarifications, naming and
so on.

We got continuous code reviews almost for free, since 100% JUnit coverage
was a project goal (I think we settled for somewhat less than 100% in the
end, but it was a noble goal).

 

[tjmadden1128]

Perhaps the "parade" convinces you, but it convinces me of the opposite.

There is a little booklet called /Wicked Problems, Righteous Solutions/ that
is worth a read. It points out that software development is a "wicked"
problem in the formal sense - its solutions are highly sensitive to initial
analysis in generally unpredictable ways. If the problem were so easily
solvable then one or another of these products would have already captured the
market.

The endless parade of cosmetic products alleged to reverse the appearance of
fine lines caused by aging does little to convince me that a Fountain of Youth
is possible, either. Generally, endless parades of products that promise the
impossible convince me only that P. T. Barnum was right in his assessment of
the public.

This is not to say that you should give up and just accept bugs. Au contraire.

No, I think these methods have merit. The company I used to work for
tended to think each one was the silver bullet that would make their
software bug-free, thus they were applied forcefully and without any
regard to whether they were needed for a project. Most people are a
little less accepting of things that are shoved down their throats. I
have yet to find a company doing TDD, though (granted, I've only had
two jobs in the last twenty years.)

Tim