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

(I think it is from an old Dilbert stripe)

Arne

I 2nd that.  It sounds like your boss picked up the term somewhere.

I agree with Patricia's comments.  Additionally, code reviews can help
you learn other easier ways of accomplishing things-because your peers
can give you suggestions on technique.

Tobi

[anecdote about a really bad programmer]

   IMHO, a diploma doesn't really mean anything. GPA is pretty
meaningless to me too. If I have a candidate fresh out of university, I'd
be more interested in seeing a list of classes she took (did she take a
compiler course? a course on AI? etc.), and then I'd ask her during the
interview about those classes (what projects did she work on for those
courses?)

   - Oliver

d. The non-comment code

Essentially, all code should be treated as communication to two
audiences with very different needs. The compiler just cares that an
identifier follows the language syntax, but "xyzzy" is just as good as
"columnNumber". The other audience is programmers, including the
author's own future self, three years later after writing a few thousand
lines more on other projects.

In addition to identifier selection, there are often multiple ways of
coding something, some of which make intent and meaning clearer than others.

Looking on the optimistic side, suppose the code is really well written,
and would be easy for another programmer to maintain. The code review is
the chance for the boss to get the good news.

Patricia

If you think that habit will ensure job security, think again. If I had
an employee who was doing what you describe here, I'd fire him on the spot.

Do your job and review the code thoroughly - the "my code is perfect" prima
donnas won't be there long enough to make your life difficult anyway.

sherm--

It can be a delicate situation, and it would depend to some extent on how  
you go about it.  Everybody needs to understand that the reviews are for  
everybody's benefit and that everybody makes mistakes, even the guru  
programmers.  Perhaps if everybody gets a chance to review somebody else's  
code it won't be so bad.  Then at least everybody experiences it from both  
sides.  If the junior programmers aren't ready to conduct a review by  
themselves, maybe they could assist a more senior colleague.  This has the  
advantage of helping them to learn from other people's mistakes as well as  
their own.  I think it also helps if the reviewer is somebody who is  
respected by the author of the code.  Also, the wounded pride that comes  
from somebody finding problems in your code can be good motivation to  
improve things for next time.

Steve McConnell has a section on formal reviews in his book, "Code  
Complete".  He also devotes a chapter to "Personal Character", which  
addresses the issue that you have just raised:

"The people who are best at programming are the people who realize how  
small their brains are. They are humble. The people who are the worst at  
programming are the people who refuse to accept the fact that their brains  
aren't equal to the task. Their egos keep them from being great  
programmers. The more you learn to compensate for your small brain, the  
better a programmer you'll be. The more humble you are, the faster you'll  
improve."

In the long run that's counter-productive.  Eventually you'll have to  
maintain that code.  It's better to catch the bad bits early before they  
get out of control.  If you spot problems with a colleague's code, just  
let them know in private - you don't have to get up in front of the whole  
team at your next meeting and declare that person to be incompetent.  
Alternatively, if you don't want to single anybody out, you can raise  
general problems in front of the whole team without referring to specific  
examples.  E.g. "I've noticed a few places where we've been over-riding  
equals without over-riding hashcode - this could cause us problems  
because...."  Then you've raised the issue and anybody who was ignorant of  
the implications will now know better without any embarrassment.

Dan.

   That's a tricky situation to be in. For what it's worth, where I work,
we feel pretty comfortable about pointing out mistakes to each other. E.g.
"You forgot a null check here", "I think it'd be clearer if you renamed
the method to 'getIntHash', rather than 'getHashInt', etc."

   We seem to realize that everyone makes mistakes, so that such
corrections are not so embarassing as to cause tension in the worksplace.
I realize, though, that not everybody is like that, and you don't always
get to choose who your coworkers are.

   Pair programming makes this a bit easier, because the feedback is
immediate, and we may console ourselves with "Well, even if he wasn't
around, I probably would have spotted that NullPointer bug eventually
anyway, given a few more seconds of staring at the code...", and the code
can be credited to both of you, so that you both share in its quality (and
its blame). But again, your boss may not be so keen on the idea of pair
programming.

   I hope, for your sake, that you're simply underestimating your
coworkers' maturity.

   I don't think anyone decides to become a programmer because they think
that'll make them famous. "Microsoft Word" is a very famous application.
Who wrote it? I have no idea. Google is a very famous web application. Who
wrote that? I don't have a clue. In fact, I suspect in both cases it
wasn't one specific person, but a team of people, further diluting the
fame of any one programmer within that team.

   I'll also point out that if the boss gives you the task of review a
piece of code, and later on, a bug is found in that piece of code, it will
be both the fault of the original author of that code for writing the bug,
and your fault for not having caught it as reviewer. If this happens once
or twice, it's not a big deal (bugs slip through; that's just how software
development is), but if the code you review is consistently buggy, your
boss may feel you are not very competent in terms of code reviewing, and
that may have adverse effects in the future.

   - Oliver

Don't you want to know your code will work correctly?  To me, that
has always been paramount.  Who cares how it got that way?

My god! where do you work? Nazi Germany?

Maybe you don't like it. The good programmers say "thank you", and
they mean it. Then they fix the code. Why, because they know it is
better a colleague finds a problem and gives one a chance to fix it,
than the boss having a word with one about the "constant lack of
quality" in the code.

It feels much worse if you are the one whos bug blew the deployment of
the software at the real big and important customer, than getting a
hint from a colleague.

Well, if a clueless newbie, e.g. one who thinks he is a programmer but
doesn't even know about code reviews, comes up with alledged bugs
every half an hour, and it turns out these are usually no bugs or
irrelevant (outside of specification), or a matter of taste, that can
certainly affect the mood.

If it is a colleague who has the respect of his team, e.g. knows what
he is doing and who doesn't brag about it if he found a bug, then  the
mood will actually get better.

And when the thing blows up, the boss will come back and ask you how
you could miss that obvious fault in the code review. And it is easy
to get caught. The original programmer has an excuse for not seeing an
obvious bug, he is to involved and immersed into the task. You have no
excuse for not spotting things.

Kid, if you are paid for reviewing some code, do it. And do it right.

And if your own code is reviewed and bugs are found, say "thank you",
and fix it. Don't act up.

You are an idiot.

You have no clue what he wants.

No, you are risking your job.

Actually, I should probably encourage you to risk your job. Maybe
you'll leave the programming profession for good. I hate to work with
slimy, artificial agreeableness people who have no spine.

www wrote :

Have you ever worked as a tester?

When you find a bug, you must go to the author of the code and inform
them that they made a mistake. You quickly learn to be really tactful.

This is the same situation.

Words really make a difference. Rather than "You idiot, what are doing
here", say "I think we need to look at this again".

And your team leader can make a big difference by not allowing personal
remarks to be said, from both sides.

If everything is kept at a professional level, it actually goes quite
well. Everyone must remember that they will be in the hot seat next
meeting.

It is a potential problem.

But a good technical leadership will be able to solve the problem.

Key points are:
1)  teach everybody that criticism is a natural way to improve and
    that as professionals they should embrace the idea
2)  keep the review in a good tone

re 1)

If they are true professionals then they will understand. If not then
maybe they should go in another direction careerwise.

re 2)

"You stupid idiot you forgot to test for null in method X !"

and

"Could we improve the code by testing for null in method X so we can
handle sitiation where ... ?"

will probably be perceived differently.

Note that in some environments (typical males below 30 only), then
the tone may be very harsh but then the one making the mistake gives
beer during Friday evening drinking and Monday morning there are
no hard feelings (maybe some hangovers).

Arne

You really don't work in a very good company/team if this is how is works.
Either your place of work really, really sucks, or you have completely
misunderstood something.

<snip>

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've never
had anyone get mad because I found a problem. Sometimes they might
want me to demonstrate the problem, or explain what was found. The
worst case I've seen is reviewing code for offshore developers who
insisted on keeping the old code in the source and commenting it out.
This led to methods that were hundreds of lines long, most of it code
that was commented out.
I've done tons of code review; as a senior developer on a project, I
reviewed code from new hires to make sure it met the coding standards
for the project, looked for any pitfalls peculiar to the project they
may not know about, and to make sure it met the request. I've done
formal reviews where a group gets together to go over changes to a
framework. I've done pair programming, although I don't find it very
productive. I get bored as the reviewer, and my attention wanders. As
the coder, I feel a little creepy having someone watch over my
shoulder. But that's just me.

Which in any organization I have been a part of would get you called
to task and hauled into a manager's or CEO's office. If your
organization does not track who did the review and ask why they did
not find the problem, then there is no point in doing it or even
pretending. In rare cases the problem would get your companies name
splashed across the media. In all cases, this undermines the
confidence of management in your team, makes the rest of the code
suspect, and loses business for the company.

Tim

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"

 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

 Regarding the attitude towards blaming:

     »Importantly, the group avoids blaming people for errors.
     The process assumes blame - and it's the process that is
     analyzed to discover why and how an error got through. At
     the same time, accountability is a team concept: no one
     person is ever solely responsible for writing or
     inspecting code. "You don't get punished for making
     errors," says Marjorie Seiter, a senior member of the
     technical staff. "If I make a mistake, and others reviewed
     my work, then I'm not alone. I'm not being blamed for this."«

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