Few days ago in a seminar one guy was talking regarding code review. At least somebody is talking about importance of code review. Most of the companies don’t have this practice. I like the way that guys had been told about code review but he was pursuing that it is Tester job. But I don’t think it is a job for the tester. Let me discuss why I think so. Let’s answer two questions
- Why? (Why we need code review )
- How? (How can we do code review)
Why?If somebody asks me why we need code review then the following things come into my mind
- Obviously get rid of some silly bugs
- To improve the quality of code and to improve the quality of developers as well
- For group learning
Let me explain the things, those I have pointed out with some examples. These examples may seems to be silly but believe me, these sort of things always happen
def update
user = User.find_by_id(params[:id])
user.name = params[:name]
user.save!
end
Here user.find_by_id may return nil value if user_id is not a valid value or don’t have the corresponding record in the database as a result user.name will raise an error and it because it will become something like nil.name. As a result it will break the whole application moreover to get rid of this there is no try catch block. This sort of silly mistakes can be found during the time of code review. And this silly bug with huge impact.
Now let us look at the following example
def completed?
is_completed = nil
if ((self.status == NOT_STARTED)
or (self.status == IN_PROGRESS))
is_completed = false
elsif (self.status == completed)
is_completed = true
else
is_completed = false
end
return is_completed
end
after code review
def completed?
return self.status == completed
end
This code is more readable, number of operation is optimized both in the sense of memory operation and if then block operation and more meaningful. So as a result your code quality has been improved and the developer has learnt it. From the next time that guy will not do the same time of mistake. As a result your developer will come to same stage.
That is a bad example I have put here but its true. On the other hand it is true that some body writes a nice block of code or impalements a new thing of course from that code block other will learn as well. So this is a team learning as well.
How?Lets talk about how we can do code review.
First let me describe the agile style of code review. After finishing the work, the developer will chose a partner for code review and inform that guy for code review and they sit together and review the code.
But in code71 we found that if we do the code review in this way then the same mistake is done by other developer. I want to say suppose I make some mistakes in this sprint but except the guy who review my code and me, nobody is aware of it and a result in the next sprint somebody else is doing the same mistake. So what we do???
On the last day of sprint we all sit together and setup a projector where we go through the code.
When we start doing this way first we made a check list to review the code you can also think about this. Let me give some key point here.
- Naming convention of classes and methods (noun, verb and whether the names are self explanatory )
- Whether the code has complex “if else” block
- Reduce the number of complex brunches to make it readable as I have show you
- Whether the behavior is distributed to the right methods or not. I mean whether one method is too long and moreover is it working with other behavior which can be the responsibility of other method.
I think initially if you go through these things that will be enough. As you go you can add more things with it. May be after couple of sprint you may find that everybody is working in a correct way.
And I hope after few sprint you will find your all team members are on the same level. And team work is extra ordinary.
