This post has already been read 1293 times!
This checklist includes basic things to look for in your code reviews, but you should also allow new
styles and patterns specific to your own team to emerge and evolve. when they do, make your own
code review checklist.
Architecture / Design
1. > Single Responsibility Principle - the idea that a class should have one and only one responsibility. You might want to apply this idea to methods as well. 2. > Open/Closed 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? 3. > Code Duplication (DRY) - don’t repeat Yourself is a common practice. One duplication is usually okay, but two are not. 4. > Squint-Test Offenses - 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’s structure ? 5. > 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. 6. > Potential Bugs - are there off-by-one errors? Will the loops terminate in the way we expect? Will they terminate at all? 7. > Error Handling - are errors handled gracefully and explicitly where necessary ? Have custom errors been added ? If so, are they useful? 8. > 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 inefficient way to locate a desired value.)
Style / Readability
1. > 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”). 2. > 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. 3. > Function Length: When a function is around 50 lines, you should consider cutting it into smaller pieces. 4. > Class Length: 300 lines is a reasonable maximum for class sizes, but under 100 lines is ideal. 5. > File Length: As the size of a file goes up, discoverability goes down. You might consider splitting any files over 1000 lines of code into smaller, more focused files. 6. > Docstrings: For complex methods or those with longer lists of arguments, is there a docstring explaining what each of the arguments does if it’s not obvious? 7. > Commented Code: sometimes you’ll want to remove any commented out lines. 8. > Number of method arguments: consider grouping methods and functions with three or more arguments in a different way. 9. > Readability: is the code easy to understand? Do I have to pause frequently during the review to decipher it ?