Code Review Check-list

Code review if one of the top software development stages in a software development process. Recently as technologies are growing, their architectures is a also growing from simpler to complex. A complex architecture is tough to maintain but it can be decomposed into simpler ones by following certain guidelines. Those guidelines are not covered in-depth here but certainly make their importance in the code review process. A code is reviewed to check if our code adhere to certain guidelines that are standard to modern code architecture specially object oriented code.

This check list is not a strict-to-follow thing but can be very helpful in a code review process.

Architectural/Design

Single Responsibility Principle

The idea that a class should have one and only one responsibility. You might want to apply the idea to methods as well

Open/Closes principle

If the language is object oriented, are the objects open for extension but closed for modification? What happens if we need to add another one of x?

Code duplication (DRY)

Don't Repeat Yourself is a common practice. One duplication is usually okay, but two are not.

Squint-Test Offences

If you squint your eyes, does the shape of this code look identical to other shapes? Are there patterns that might indicate other problems in the code structure?

The Boy Scout Rule

If you find code that's messy, don't just add a few lines and leave. Leave the code cleaner than you found it.

Potential Bugs

Are there off-by-one errors? Will the loops terminate the way we expect? Will they terminate at all?

Error Handling

Are errors handled gracefully and explicitly where necessary? Have custom errors been added? If so, are they useful?

Efficiency

If there's an algorithm in the code, is it using an efficient implementation? (e.g. iterating over a list of keys in a dictionary is an efficient way to locate a desired value.)

Style/Readability

Method names

Methods should have names that reveal the intent of the API while fitting into the idioms of your language and not using more text than is necessary (e.g. it's not "send_http_data" it's "post_twitter_status").

Variable names 

Foo, bar, e: these names are not useful for data structures. Be as verbose as you need (depending on the language). Expressive variable names make code easier to understand.

Function length

When a function is around 50 lines, you should consider cutting it into pieces.

Class length

300 lines is a reasonable maximum for class sizes, but under 100 lines is ideal.

File length

As the size of file goes up, discover-ability goes down. You might consider splitting any files over 1000 lines of code into smaller, more focused files.

Docstrings

For complex methods or those with longer list of arguments, is there a docstring explaining what each of the arguments does if it's not obvious?

Commented code

Sometimes you will want to remove any commented out lines.

Number of method arguments

Consider grouping methods and functions with three or more arguments in a different way.

Readability 

Is the code easier to understand? Do I have to pause frequently during the review to decipher it?


Communicating your review

Ask questions

How does this method work? If this requirement changes, what else would have to change? How could we make this more maintainable?

Complaint/reinforce good practice

One of the most important parts of the code review is to reward developers for growth and effort. Few things feel better than getting praise from the peer. Try to offer as many positive comments as possible.

Discuss in person for more detailed points.

On occasion, a recommended architectural change might be large enough that it's easier to discuss it in person rather than in the comments. Similarly, if discussing a point and it goes back and forth, try to pick it up in person and finish out the discussion.

Explain reasoning

It's often best both to ask if there is a better alternative and justify why a problem is worth fixing. Sometimes it can feel like the changes suggested can seem nit-picky without context or explanation.

Make it about the code, not the person

It's easy to take feedback from code reviews personally, especially if we take pride in our work. It's best to make discussions about the code rather than about the developer. It lowers resistance and it's not about the developer anyway. It's about improving the quality of the code.

Suggest importance of fixes

Try to offer many suggestions, not all of which need to be acted upon. Clarifying if an item is important to fix before it can be considered done is useful for both reviewer and the reviewee. It makes the result of a review clear and actionable.