Billy wrote:
> Ed Prochak wrote:
>
> > If participants enter with the idea of beating up the coder, then who's
> > suprised at the coder getting bruised? This should not be like ORAL
> > exams for a doctorate degree. The goal is better code and some team
> > communication.
>
> And if the coders "refuse" to comply - to understand the basics, RTFM,
> achieve enlightment, etc?
>
> More than once I've had the occassion where new technology is used and
> developers insisting on using old concepts. It tends to get quite
> confrontational at times. That bit about leading a horse to water...

That's exactly the point. It is their code. If it fails to meet
functionality or performance specs, then your manager can put on the
pressure. You as a reviewer cannot and should not pressure the
developer.


>
> I do not mind if the issues is like demonstrating why soft parses are
> better than hard parses, or no parses (statement handle re-use) is
> better than soft parses.. or what the bindless SQL do versus bind SQL.
> Or the advanatages of bulk processing.
>
> But what when the issue is a conceptual one and the counter arguments
> is based on ignorance of basic Oracle concepts? Refer them to the
> manual? Which they do not bother to read - ever? Spend hours and hours
> building test cases to show basic fundemental concepts? Thus I do tend
> at times reach for the lead pipe as I refuse to waste my time (I also
> have deadlines) with developers that refuses to understand fundamental
> concepts after I've explained in brief the concept with references to
> applicable Oracle manuals and documentation. (e.g. why ref cursors for
> doing HTMLDB reports is non-workable idea)

Reviewers should never carry lead pipes! In the review you note the
problem or location for possible enhancement and move on. You do not go
into the details of the alternate solutions.
>
> I think it is easier to deal with younger guys. They are still clean
> slates and can be taught. Much more difficult with older developers
> that believe that their development Kung Fu is of such a high standard
> that they know better than you.

that's a team/personnel/management issue, NOT a review issue.
>
> > A little training of participants goes a long way toward preventing the
> > negative results. Just the simeple reminder that it is a CODE review,
> > not a programmer review helps. Everyone, including the coder needs to
> > be reminded occasionally.
>
> Agree. But the Oracle University PL/SQL courses for example, seems to
> me very basic and does not teach/re-inforce fundemental programming and
> design techniques.
>
Note: I was talking about being trained in the review process, not
training in programming laqnguages.

> > One rule (of many) we used at a place that did Inspections is that the
> > coder owns the code. So even if reviewers say, change this, reformat
> > that, etc., the coder has to respond but the response may be to ignore
> > the advice. This empowerment reduces the stress levels all around.
>
> I have a problem with that - as I would sit in meetings with management
> and operations and have to explain why there are poor performance. Or
> deal with issues like snapshots too old, deadlocks and so on..

Why isn't the coder in that meeting instead of you? This is where the
owner of the code has to start explaining why he failed to follow
suggestions from the review. You already did the CYA move in the
review, pointing out the potential problems (Your team does retain
review minutes, right?).
>
> Is it too much to expect a certain quality in the code that is put into
> production?

No, that's why we do code reviews. But you clearly need support,
primarily from your manager. The review process isn't your problem,
and using the review as a ranting session is just a BAD IDEA. Obviously
your teammates are less interested in producing quality code than you
are. That's a rough place to be.

>
> > And a final point. There are many ways of doing these reviews. Simple
> > desk checks, Execution reviews, unstructured reviews, structured
> > reviews, and software inspections. The unstructured reviews most easily
> > break down to beat-up-the-developer sessions. Whatever you do, the
> > process needs some clear guidelines AND practice.
>
> Agree with you on that Ed. In fact, I dislike the idea of a code review
> entirely. If it comes down to that I would rather want to deal with the
> devloper alone on a personal level - not in a peer situation where egos
> are at stake.
>
> --
> Billy

Done right, reviews should remove the ego issues, or at least isolate
them outside the review meeting. A one-on-one desk check is one of the
simplest reviews, but it is a coarse filter. Still it's better than no
filter at all.

Please don't take offense, Billy, but you seem to take the problems
personally, even when it is not your code. Keep the horse proverb in
mind a little more often. It becomes almost a political process, a
matter of persuassion, to bring your temmates around. Again I say your
problem is not with reviews, but with the team. I hope it gets better.

Ed

Ed Prochak wrote:

> That's exactly the point. It is their code. If it fails to meet
> functionality or performance specs, then your manager can put on the
> pressure. You as a reviewer cannot and should not pressure the
> developer.

And when they spin technical yarns and the manager does not have the
savvy to understand what is fact and what is not?

Remember that managers are suppose to manage time, people and money -
no technical skills needed.

> Why isn't the coder in that meeting instead of you? This is where the
> owner of the code has to start explaining why he failed to follow
> suggestions from the review. You already did the CYA move in the
> review, pointing out the potential problems (Your team does retain
> review minutes, right?).

Souds good. But there are often Chinese Walls between operations and
development.

> Please don't take offense, Billy, but you seem to take the problems
> personally, even when it is not your code.

Only in production when data "disappears" and the developers firstly
blames Oracle for it.. only later to discovered that is is a -very- bad
idea to supress exceptions in a trigger. :-)

> Keep the horse proverb in
> mind a little more often. It becomes almost a political process, a
> matter of persuassion, to bring your temmates around. Again I say your
> problem is not with reviews, but with the team. I hope it gets better.

Actually Ed, I'm playing more devil's advocate than anything else
(though the issues I raised I have run into personally though the
years). Code reviews only work in my experience when all parties buy
into it - but developers seldom do as it becomes an ego thing
(especially when the review is in his peer group) where you need to
prove to the developer why ABC is better than his XYZ.

At the current place I'm contracting, we do not have code reviews at
all. Pretty difficult to implement something like this.. I'm dealing
with full time developers, external contractors, and senior
technical/specialist staff that are forced to write code (prototypes
that invariable winds up in production due to marketing windows of
oppertunities). They not only report to different managers, but work
for different divisions. Thus enforcing any type of coding standards,
is mission impossible.

But the proper way to do things are slowly filtering through. How many
times do one have to bump one's head before seeing the light? :-)

--
Billy

Billy wrote:
> Ed Prochak wrote:
>
> > That's exactly the point. It is their code. If it fails to meet
> > functionality or performance specs, then your manager can put on the
> > pressure. You as a reviewer cannot and should not pressure the
> > developer.
>
> And when they spin technical yarns and the manager does not have the
> savvy to understand what is fact and what is not?

You mentioned performance issues. If the routine isn't returning the
data for 5seconds when this is for a web page that needs to display in
under 2seconds Isn't the manager going to question that? Then when the
dinosaur coder says he cannot make it faster, but you then propose that
you can, and then you really do it. Who is the manager going to believe
next time?

But like I said, your problem is NOT with the review process.
>
> Remember that managers are suppose to manage time, people and money -
> no technical skills needed.

maybe not coding skills, but communication skills are required. Being
able to identify that the technology has changed and the old
programming solutions are not cutting is falls in that category. But
again, we are outside the review.
>
> > Why isn't the coder in that meeting instead of you? This is where the
> > owner of the code has to start explaining why he failed to follow
> > suggestions from the review. You already did the CYA move in the
> > review, pointing out the potential problems (Your team does retain
> > review minutes, right?).
>
> Souds good. But there are often Chinese Walls between operations and
> development.

But which side of the wall are you on that you are in developer reviews
and in operations meetings.
>
> > Please don't take offense, Billy, but you seem to take the problems
> > personally, even when it is not your code.
>
> Only in production when data "disappears" and the developers firstly
> blames Oracle for it.. only later to discovered that is is a -very- bad
> idea to supress exceptions in a trigger. :-)

which shouild have been identified in a code review.
>
> > Keep the horse proverb in
> > mind a little more often. It becomes almost a political process, a
> > matter of persuassion, to bring your temmates around. Again I say your
> > problem is not with reviews, but with the team. I hope it gets better.
>
> Actually Ed, I'm playing more devil's advocate than anything else
> (though the issues I raised I have run into personally though the
> years). Code reviews only work in my experience when all parties buy
> into it - but developers seldom do as it becomes an ego thing
> (especially when the review is in his peer group) where you need to
> prove to the developer why ABC is better than his XYZ.

And sometimes you need to give the developer room to fail. When the
data disappears and the VP is chewing him out, he'll get the message.

But you are right, it works best when all are willing to follow the
idea. I've been trying to get code reviews where I am right now but for
severl reasons, it's not flying. Thing is I know that I develop better
code when there is a review process. Here it's the old code & test.

>
> At the current place I'm contracting, we do not have code reviews at
> all. Pretty difficult to implement something like this.. I'm dealing
> with full time developers, external contractors, and senior
> technical/specialist staff that are forced to write code (prototypes
> that invariable winds up in production due to marketing windows of
> oppertunities). They not only report to different managers, but work
> for different divisions. Thus enforcing any type of coding standards,
> is mission impossible.

I understand, I really do.
>
> But the proper way to do things are slowly filtering through. How many
> times do one have to bump one's head before seeing the light? :-)
>
> --
> Billy

Some of those people have really thick skulls and short memories. The
real trouble is not that they bump their heads, but they confuse the
absense of pain (after the mess is cleaned up) with real pleasure
(avoiding the mess to begin with).

Nice talking with you. You made some good points about development
teams. That's a whole 'nother topic from reviews. If you want some good
reading about it, look up some of the books by Demarco & Lister. And of
course the Dilbert Principle.

good luck.
ed

Billy wrote:
> DA Morgan wrote:
>
>>An excellent methodology. I plan to flatter you by copying it the next
>>time I get the opportunity: Brilliant.
>
>
> I'm more flattered by free beer coupons.. <hint> <hint>
>
> ;-)
> --
> Billy

So why weren't you at OpenWorld?

If you'll be at UKOUG 29-October - 2-November I'll buy you all the
swill you can quaff.
--
Daniel A. Morgan
http://www.psoug.org
damorgan@x.washington.edu
(replace x with u to respond)