quarta-feira, 6 de outubro de 2010

Honey! It's not what it looks like: CODE REVIEW

Today I will start a new brand of posts: "Honey! It's not what it looks like". The idea here is to present some concepts about terms commonly used, but sometime misunderstood or just not clear enough to give a common ground for a good conversation.

Some confusion could raise when we use those kind of terms, you may use them expecting something and you get a completely different stuff; it is like in the every day life, you expect something in the other hand you get a raining of popcorn in your head, a dog barking or running at you and so on. The best we can do is go get a common ground about the terms, so every time that you see "Honey! It's not what it looks like" it will be me trying to discuss some of those terms.

As starter I will stick with code review. When someone comes to you and say: we need to do code review, what does it means? What are the expected outcomes from the code review? In a conceptual world which software engineering discipline is it included?
Let's just set the ground, every word or better group of word could have a locale meaning, I am not saying that those meanings are wrong, absolutely not, just saying that when those meanings are not clear, maybe you will get what you expect, but probably you will end up with popcorn in your head.

Looking at the Pressman[1] definition for FTR: "A formal technical review is a software quality assurance activity performed by software engineers (and others). The objectives of the FTR are (1) to uncover errors in function, logic, or implementation for any representation of the software; (2) to verify that the software under review meets its requirements; (3) to ensure that the software has been represented according to predefined standards; (4) to achieve software that is developed in a uniform manner; and (5) to make projects more manageable.", let's focus at item 1: the objective is to uncover errors in function, logic or implementation of any representation of the software, and the code is a representation of the software, as much a class diagram would be or a architectural design.
So from the Pressman definition we can say that a code review is a FTR applied at the code, which is a representation of the software, so from now on we can exchange the terms code review and FTR.
With the bases set: code review is a FTR, we can look at the other objectives of a code review; verify that the software meets the requirements, guarantee software standards, achieve development uniform and make it more manageable. Well that is perfect, right? With a simple review I can achieve all of it?

The answer is: yes and no, that is why: in order to achieve the desired output we need to have a proper input, for example the item 2. Do my software meets its requirements, yes a review could give you this answer, but to get the answer you need to have a proper requirement, otherwise how can I review my code? So when you ask for a code review and expect to have a item 2 attempted you need to give the proper input, and if you don't have a proper input, well then you have a problem.

I will not extend the discussing about the other items, but all of them require proper input to give you a proper output. I am not saying that the proper input is the 100% guarantee of a perfect output, but it is a good trail. In the other hand, it is not mandatory that all the output could be expected. You could say that I just want item 3 or item 4 or even another thing, but that need to be said.

The other element that you need to consider when you say –"I am going to do code review", it is how you are going to do that. According to Pressman[1] a FTR " is actually a class of reviews that includes walkthroughs, inspections, round-robin reviews and other small group technical assessments of software. Each FTR is conducted as a meeting and will be successful only if it is properly planned, controlled, and attended", are you planning to do all of it? If you are not then you need to understand that not all the outcomes from a FTR will be there for you or it will be valid.

Let's suppose that you don't have a meeting for the code review, how will you be able to achieve a development uniform knowledge in your team? A FTR when done by different people is a powerful tool to spread knowledge, sure we have alternatives like pair programming, but the alternatives should be investigated, and it is absolutely true that you need to do something, it doesn't come for free (again you don't do it properly the dog will bark at you).

There is alternatives to a FTR, to be more specific a FTR at the code exists, like the previous referenced pair programming, and there are others (you will find some references to other articles and web pages at the end of the post).
The important issue to consider is: what do I want to achieve with code review? Meet the requirement, standard code, spread of knowledge, manage my projects, etc; and from this point you need to start filling what those words mean. And remember, the experience proved that a code review done by more than one people with different knowledge about the problem is much more effective than a single person reviewer (ok I know that I should put some proper article reference here, so if you want I can bring it).

And before the end I would like to point to another detail concerning the FTR; where the FTR is placed in the Software Engineering disciplines. It is an instrument of Quality Assurance.
Some could say that in XP the pair programming is done by the developers and it is a code review; that is true, but it is still a Quality Assurance instrument. But why is it important? It is important because it shows what kind of purpose this tool has, it is not a testing tool, and it is one of the instruments to guarantee the quality of the code into the process, you should remember that one of the purposes of the Quality Assurance is to guarantee the quality of the product through a better process.
In this case the better process is achieved, for example, with the code knowledge spread across the developers. At the end while you are doing FTR you are also improving your software process, but it should be done properly and if this idea in mind.

As promised here it is a small list of articles about people discussing FTR, mainly code review, plus a website with a check list for code review[5] (could be a start for this kind of checklist):

[1] R.S. Pressman, Software Engineering – A practitioners's approach, MCGraw-Hill, 2001.
[2] T. Stalhane, C. Kutay, H. Al-Kilidar, R. Jeffery, Teaching the Process of Code Review, Proceedings of the 2004 Australian Software Engineering Conference (ASWEC’04), 2004.
[3] A. Vardhan, Learning to verify system, 2006.
[4] A. Harel, Prof. E. Kantorowitz, Estimating the Number of Faults Remaining in Software Code Documents Inspected with Iterative Code Reviews, IEEE International Conference on Software - Science, Technology & Engineering (SwSTE'05), 2005.
[5] Macadamian, Code Review Checklist, http://www.macadamian.com/insight/best_practices_detail/code_review_checklist/, 2010.

So remember that from time to time we need to recycle your dictionary, your common sense. Said that I will let you with the song, Time after Time, from the Brazilian band Dr. Sin.

2 comentários:

Fábio disse...

sehr gut mein freund ! inklusiv die rock band ...lol

Link d'Alekine disse...

Hahahah as you said it was missing the Dr. Sin in this band list.

I will try to keep with this kind of post, maybe it could help with some enlightenment :-D

By the way I have all those pdfs, with you want any...