Being prepared for a code review as a developer?

Question

I am looking for some ideas here.

I read the article How should code reviews be Carried Out and Code Reviews, what are the advantages? which were very informative but I still need more clarity on the question below.

My Question is,

1. Being the target developer, can you suggest some best practices a developer can incorporate before his code is going get reviewed.

   • Currently I practice the following methods

     • PPT for a logical flow
     • Detailed comments.

Issue: Even though I have implemented the above practices, they do not help on the review. The problem I faced is, when certain logic is referred, I keep searching for the implementation and the flow and too much time is wasted in the process and I get on people’s nerve.

I think a lot of developers would be going through what I am going through as well.

Answer 1

So based on details OP provided, it sounds like the question is, "how do I learn my own code so that when asked to find X or explain Y, I'm able to respond quickly."

Few suggestions that I can think of:

• When coding, you need to take the time to learn and understand your own code. This could be what your TL is trying to get across to you in not so many words. Being a TL on the current project, I've done a lot of code reviews in the last 11 months and I do notice a practice of some developers to search for "example code" either in our own code base, or somewhere else (google, etc...) and copy/paste it in. Personally, I can't stand it because while their code passes the simple unit tests, they do not understand what it is actually doing, so we are never guaranteed that there isn't some boundary case or an expected failure condition that could occur.

• As a corollary to previous statement, if you have to copy/paste, try to only copy/paste the code YOU have previously written and that you understand. It is certainly ok to "borrow" other people's idea but in that case, rewrite their code line by line because as you are writing it, you will gain better understanding into what it does. If you are using external APIs, even if you have an example that uses that API, take a few minutes anyway to find a reference and learn how that API works. Don't just assume that if it worked before, it will also work in your situation.

• Read up and learn to love the DRY principle. A lot of times what you are tempted to copy/paste could be placed in a common location (separate function, separate class, separate library...)

• Read up and learn to love SOLID principles and while you are at it, review KISS which was already mentioned by mouviciel. These principles are all oriented at producing very concise, clean and modular code. If you have large classes and large functions within those, it is clearly going to be much harder to find things and on top of that try to explain what the code does. On the other hand, if you follow (or at least try to follow) SRP and make each class/function responsible for one thing only, your code will be small and very readable.

• Pick up a copy of Clean Code. Very good book. It talks about writing code that is self explanatory and easy to read, maintain and extend. If you practice writing code that is easy to read, you shouldn't have problems reading your own code in the code reviews. And this is the funny part, I've asked people to read their own code or simply tell me what the variables were representing and they couldn't answer even though they wrote that code (brand new classes, not legacy) only a week ago. Good naming goes a long way.

• If after all the simplification and refactoring, you still have a function that has to perform some kind of algorithm which is not very apparent, take the time and write a comment block in that function explaining the algorithm. Not only will it be helpful when you have to modify that function 2 months from now, but if you get ambushed in a code review, you'd be able to simply read back what you wrote.

• If after all the items above, do you still find yourself in trouble? are you new to the team and asked to work with a lot of legacy code? In that case, it could be that your TL is being an A$$ and you could be proactive by asking him before the meeting to go easy and not waste the time of everyone involved. When new people join a team, TL needs to have enough patience because working in a new platform, new product, new people, new environment takes a lot of concentration from a new person, and that person will be missing some details in the beginning. Works as Designed and your TL should just accept that.

• If after all items above, you still feel that you have horrible code reviews. Talk to your TL. Sometimes people feel bad because of the nature of code review meetings when in fact TL is perfectly happy with you. When I do code reviews, my goal is to highlight what needs to be changed, make sure you understand the changes and move on. A lot of times I don't have time to be polite and some people get defensive and attempt to answer every single one of my comments. In those situations code review meeting grinds to a halt so I tend to interrupt them and move on. Generally, after the meeting I would talk to the new guys to make sure they understand the process and that it is nothing personal. After few code reviews people are generally much more comfortable.

Answer 2

Practices vary, but in my experience:

• Don't do anything special to the code. It's natural to spiff up your code a little more when you learn that it's going to be reviewed, and there's no harm in fixing obvious things like spelling mistakes and such. But don't go in and add a lot of detailed comments or otherwise change the code just because it's scheduled for review.

• Code is prepared and distributed to reviewers well in advance of the review. This is usually done by a neutral third party, probably the code review facilitator. If printed out, the code should be small enough that lines aren't wrapped too often, but large enough that everyone can read it easily. Print it in landscape format if that's what it takes.

• Code should be printed or displayed with line numbers. Preferably, the number should continue from one file to the next. It's so much easier to refer to "line 3502" than "line 238 of foo.c", and having the numbers lets everyone talk about specific lines without wasting time finding those lines.

• There should definitely be a facilitator, btw. His or her job is to keep the review from getting bogged down in minutia, prevent it from getting personal or heated, and strictly limit the length of the review.

• As the author, you should review the code yourself before the review meeting. Write down the changes you'd suggest if this were someone else's code. This jogs your memory of code that you might not have looked at in a few days, and it also helps you practice looking at your own code with a critical eye. After you've been through a few reviews, both as reviewer and as author, you'll find that your own notes will more closely match those of the rest of the group.

• Be prepared to take notes during the review. This shouldn't be your main concern -- someone else should be recording the action items that the group agrees on so that you can focus on explaining the code and listening to the feedback. But there will be times when you get some valuable feedback that isn't an action item, and you should right such things down as they occur.

• Remember that it's not personal. It's hard to avoid feeling (and acting) defensive during a review. It's fine to explain your code if you think it was misunderstood, but more than anything else try to just listen.

Answer 3

One more thing to add to the other answers: to make formal code reviewers easier, conduct LOTS of informal code reviews! For instance:

"Hey Bob, can I show you how I implemented the foo() function?" "Hey Steve, can you take a look at this class diagram and let me know what you think?" "Hey Karen, can you help me think through this problem? I think I've got a good solution, but I could use your help..."

Make this a regular habit. When you involve your coworkers early in the design process, you:

• Build relationships
• Gain new insights into the problem
• Improve your ability to explain the problem / solution at hand
• Save time later in formal code reviews