«

Hi,

My boss just told me a concept I didn't know. (I am middle level Java
programmer.) -- "code review".

According to what I heard, "code review" is somebody reads the thousands
lines of code written by other person and try to find if there are some
errors (logic errors, I guess, since the code at least can be compiled
and run).

I feel this is crazy!!! Since the reviewer has to "read" the original
code author's mind and make sure the code does what the author wants and
no hidden surprises! How this could be possible?! This would be
extremely time consuming and nobody knows better about the code than the
author.

My boss says this is very common practice in software engineer development.

Is this true? Or my understanding from my boss is wrong?

www <> writes:
>According to what I heard, "code review" is somebody reads the thousands

Ever heard of search engines or Wikipedia?

www said:
> Is this true? Or my understanding from my boss is wrong?

Why not ask your boss to clarify exactly what he means. Often management types
use terms they've picked up without having a clue to what they mean at all.

D.
--
Fermat was right.

www wrote:
> Hi,
>
> My boss just told me a concept I didn't know. (I am middle level Java
> programmer.) -- "code review".
>
> According to what I heard, "code review" is somebody reads the thousands
> lines of code written by other person and try to find if there are some
> errors (logic errors, I guess, since the code at least can be compiled
> and run).
>
> I feel this is crazy!!! Since the reviewer has to "read" the original
> code author's mind and make sure the code does what the author wants and
> no hidden surprises! How this could be possible?! This would be
> extremely time consuming and nobody knows better about the code than the
> author.
>
> My boss says this is very common practice in software engineer development.
>
> Is this true? Or my understanding from my boss is wrong?

I've only participated in informal code reviews, in which the objective
is not to look at every line, but to find the lines that should be
looked at.

There is often a mix of two high level objectives:

1. General search for errors.

2. Verification of code quality.

Both aspects should focus on problems that cannot be detected by the
software development tools.

For example, I would spend far more time on synchronization and
multi-threading issues than on aspects where bugs would be found in the
first few minutes of testing. There is no point in spending human time
checking whether the indentation is correct, but no computer program can
evaluate whether identifiers are meaningful.

Remember that readability is an important aspect of code quality. Code
that can only be maintained by the author is of limited value in most
organizations. I don't think there is any automated test for readability
that works anywhere near as well as seeing what happens when a
non-author programmer tries to read the code.

This is a good time to review your documentation imagining another
programmer trying to understand your code.

Patricia

On May 10, 7:30 am, www <w...@nospam.com> wrote:
> Hi,
>
> My boss just told me a concept I didn't know. (I am middle level Java
> programmer.) -- "code review".
>
> According to what I heard, "code review" is somebody reads the thousands
> lines of code written by other person and try to find if there are some
> errors (logic errors, I guess, since the code at least can be compiled
> and run).
The code review is more about verifying that the design is correct,
but it does often find logic errors as well.
>
> I feel this is crazy!!! Since the reviewer has to "read" the original
> code author's mind and make sure the code does what the author wants and
> no hidden surprises! How this could be possible?! This would be
> extremely time consuming and nobody knows better about the code than the
> author.

That's not necessarily true. An author might have a good feel for the
code at the immediate moment, but what happens down the road a year or
two when you haven't thought of it for that long... You'll have to
time travel and read your own mind... But not really, if you write
well designed code, the code will convey your intentions (such as by
method name, variable name, etc...) and what you actually did (its not
difficult to step through a method mentally). A good code reviewer
would ask you questions like "You named this method getBars, when if
fact what you seem to do is get wibbles. Is it named appropriately?"

>
> My boss says this is very common practice in software engineer development.
>
> Is this true? Or my understanding from my boss is wrong?
Code reviews are common practice and a good idea. You're boss is
correct, but your thoughts are not.

I wouldn't call yourself a middle level Java programmer quite yet. I
consider middle level someone who can convey meaning in there symbol
names, as well as someone who knows what a code review is.

> According to what I heard, "code review" is somebody reads the thousands
> lines of code written by other person and try to find if there are some
> errors (logic errors, I guess, since the code at least can be compiled
> and run).

It's not just about parsing thousands of line of code but, yup, you
can do that. And it's *crazy* what an "OK" programmer will find in
code produced by some not-so-OK-yet programmer.

I've done "dumb eye ball searching" code review (literally reading
lots of lines of codes one by one) and what you find is usually
pathetic.

I've been working for a startup where a programmer had a "working"
program in which a method had 2000 lines of code.

2000 lines of nested "if / then / else" with basically the same
code copy/pasted and nested and nested several times.

(this "method" ended up being rewritten in about 5 methods /
130 lines of codes)

Crazy.

And that guy had a diploma and passed the interview...

Worst I've seen was probably in some source code
for a medical appliance where the manipulation of
dates showed a complete non-understanding of both
programming and the API available.


> I feel this is crazy!!! Since the reviewer has to "read" the original
> code author's mind and make sure the code does what the author wants and
> no hidden surprises! How this could be possible?!

Well for a start the reviewer will see if the code is readable
at all. If only the author can read what some piece of code
does then the company is in big trouble.

Note that I won't call names but there's a specific
part of the programming community that do use that
age old trick of "writing unreadable code to keep their
job".

And it appears quite obviously when doing code
reviews. Companies don't need such jerks.


> ... and nobody knows better about the code than the
> author.

But lots of programmers knows for sure a better
way to do what the code of that author does.

Been there, done that.

"www" <> wrote in message
news:f1va9a$ch3$...
>
> According to what I heard, "code review" is somebody reads the thousands
> lines of code written by other person and try to find if there are some
> errors (logic errors, I guess, since the code at least can be compiled
> and run).
>
> I feel this is crazy!!! Since the reviewer has to "read" the original
> code author's mind and make sure the code does what the author wants and
> no hidden surprises! How this could be possible?! This would be
> extremely time consuming and nobody knows better about the code than the
> author.

If the review has to read minds, and this is impossible given the
level of documentation and choice of identifiers in your code, then your
code is probably poorly written.

Consider the following code snippet:

public int a(int b) {
return b;
}

Does this do what the author intended? Are there any hidden surprises?
It's impossible to tell, because the author did not write any
documentation, and chose meaningless identifier names. Contrast with this
code:

public int hashInt(int intToHash) {
/*
* Implementation is just to return the same
* int. This is a perfect hash.
*/
return intToHash;
}

Now, thanks the the name of the identifiers, and the comments, we can
guess what the author was trying to do, and check that it does in fact do
what the author intended.

Code reviews correctly would flag the first example as needing to be
rewritten.

>
> My boss says this is very common practice in software engineer
> development.
>
> Is this true? Or my understanding from my boss is wrong?

It's common. I don't know about "very" common, but it's common enough
that I'd expect someone referring to themselves as a "middle level
programmer" (whether Java or some other language) to know what the term
"code review" refers to if I brought it up.

As an aside, to help re-assess your self-evaluation, I'd also expect
someone referring to themselves as a middle level programmer to be
familiar with the following terms. If you're not familiar with them, you
may wish to do some more reading.

* Design patterns
* Extreme Programming
* Test Driven Development
* Unit Testing
* Black Box Testing
* Model View Controller
* Singleton
* Source Version Control
* Pair Programming
* KISS
* UML
* Class Diagram
* Sequence Diagram

Note that for the terms that refer to methodologies, I don't expect a
middle level programmer to actually *practice* all of the above
methodologies, but rather that they should be familiar with what the terms
refer to.

- Oliver

<> wrote in message
news: oups.com...
>
[anecdote about a really bad programmer]
>
> And that guy had a diploma and passed the interview...

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

On Thu, 10 May 2007 10:30:02 -0400, www <> wrote:

>Hi,
>
>My boss just told me a concept I didn't know. (I am middle level Java
>programmer.) -- "code review".
>
>According to what I heard, "code review" is somebody reads the thousands
>lines of code written by other person and try to find if there are some
>errors (logic errors, I guess, since the code at least can be compiled
>and run).
>
>I feel this is crazy!!! Since the reviewer has to "read" the original
>code author's mind and make sure the code does what the author wants and
>no hidden surprises! How this could be possible?! This would be
>extremely time consuming and nobody knows better about the code than the
>author.
>
>My boss says this is very common practice in software engineer development.
>
>Is this true? Or my understanding from my boss is wrong?
Think of a code review as an opportunity for you to improve your
programming skills. Your boss is letting you take some of your
colleagues time to comment on your code. That lets you pick their
brains as to how you could code better. How much would it cost you to
hire a consultant to look at your code and suggest ways to improve it?

rossum

www <> writes:

> I feel this is crazy!!! Since the reviewer has to "read" the original
> code author's mind and make sure the code does what the author wants
> and no hidden surprises!

If ESP is required to divine the code's purpose, that's already the first
sign of a problem. There should be:

a. A written specification
b. Unit tests
c. Comments within the code itself

Furthermore, it shouldn't be delayed until thousands of lines of code have
already been written. It's an ongoing, continuous process, not a checklist
item that must be checked once before shipping. In "pair programming", this
actually involves two programmers sitting in front of one keyboard, working
together and reviewing one another's code.

> How this could be possible?! This would be
> extremely time consuming and nobody knows better about the code than
> the author.

What if the author gets hit by a meteor? Code *must* be readable and main-
tainable to be useful. If the reviewer can't read the code, it fails the
code review.

> My boss says this is very common practice in software engineer development.

Your boss is correct.

sherm--

--
Web Hosting by West Virginians, for West Virginians: http://wv-www.net
Cocoa programming in Perl: http://camelbones.sourceforge.net