0

Can an effective code review be performed without first understanding the intent behind the code?

6 Answers 6

8

One of the main reasons for code review is "does it do what it is supposed to do?" Without knowing requirements (and, thus, intent) for the code you can only do a formal analysis of the code (something you can often automate using static analysis tools).

When the reviewer is not familiar with the requirements, you also run the risk of the reviewed making up real or fictitious reasoning on the fly to justify their decisions, which can be hugely frustrating for the reviewer (he's running against a rubber wall).

So, the answer is: yes, you can - but you lose lots of the inherent benefits and you run the risk of creating a really frustrating experience for both reviewers and reviewed.

4

It’s limited. Now the code will go through QA so if it doesn’t do what it is supposed to do it will be stopped, and I could assume that you make mistakes, but don’t do something outright stupid. A review still can check that you are not damaging the code base, do not introduce security risks and do on.

4

Somewhat. General code style and approach is something that you don't need to know context for.

However, it's perfectly possible to write style-compliant code that doesn't achieve the goal; and without knowing the goal, it's impossible to review if it has been achieved.

There is also an argument to be made that by letting your code be reviewed by someone who doesn't know the goal of the changes; you can test how readable and self-documenting your code is.

That is certainly true, but any experienced developer/reviewer should be able to review this without needing to do it "blind".

When you sum it all up, I am of the opinion that the benefit of doing a true blind review (i.e. is this code readable for any developer?) does not offset the loss of sensibility checking that an aware reviewer can perform (i.e. has the goal been achieved?).

In other words:

No code review < Blind code review < Aware code review

It's not better, but it's still better than nothing.

3

Why would you expend time & money reviewing something that you don't know is even fit for purpose?

Review this:

// Calculate area.
a=width*height;

A rectangle? Is it a square with over-specified parameters/interface? An ellipse with a bug in it?

How about this?

List<string> clientCodes;

hmmm, could this be ok? Are clientCodes unique and should be a dictionary? Do I need to find them quickly or doesn't it matter? How many are there? 10 to 20 or 10,000,000 to 20,000,0000? Does it need to be used in a multi-threaded environment? Are there constraints on the contents?

Almost pointless.

1

Yes a code review can be done without understanding the intent of the change. Knowing nothing about the repository itself makes reviewing code impossible, because you don't know any of the important decisions the code is supposed to follow. Code reviews are not about correctness of the code, that should be assumed generally. Code reviews are about patterns and practices. The more important thing is that code is written in places where it belongs, because that is far harder to fix later in most cases than an incorrect implementation. If an area calculation was incorrectly made as:

// Calculate area. a=width+height;

it's trivial to made the + a *. What isn't trivial is if this calculation is being done in the UI for a specific page(s) and not in a business logic layer, or worse done in both independently. The latter causes far more bugs and inconsistent behavior over the life of a system than a wrong calculation ever will.

That's not to say all comments about correctness are bad, but you don't need to know the details of a change to question if x.FooID==y.BarID is potentially an issue or could use a comment to explain comparing two apparently different values. A for loop that skips the first or last element could be intentional, but noting it in a review is still good. Nested if conditions (or any at all in some cases) should always be called out. These sorts of issues are also bad practices.

1

A code review can check that the code:

  1. Conforms to the shops conventions
  2. Does what the developer intended for it to do
  3. Does what the product owner intended for it to do

Well written code will pass 1 and 2 fairly well. When 2 and 3 disagree you're not going to find that out from the code. Well written code can still be misapplied. You need to know more. Requirements knowledge can help with that. 3 being the most critical, it's worth finding some way to ensure 2 and 3 agree.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.