While writing the post on code reviews, it occurred to me that I could also write about the things I look at while doing it and provide you with a checklist that you could use to perform your own reviews. I will try to explain why I find each of these points important, but you can use or modify the list however it suits you.

So, without further ado, here is the list.

1. Code formatting – in general, this is something that you should let your IDE do for you and the whole team should have a common code formatter configuration file to use. It is important that everybody uses it, because it will make the code looks consistent, and as such much more readable. If I notice anything that looks as if the code formatter is not setup, I’ll ask the author of the code to configure the formatter, and setup the Save actions so that the code is automatically formatted on each save. It is a configure once and forget action that does not take much effort, but helps a lot.
2. Spelling – no matter how trivial this sounds, having a spelling error in a name of a class/method or a variable can make it pretty difficult to find it. This can become specially frustrating if you know the name of the class you look for, but are not aware of the spelling error. All modern IDEs provide you with a keyboard shortcut to directly jump to a class, or a method inside it, so if there is a spelling error it may be a difficult task to find it. All IDEs have an integrated spell checker, so it really is easy to write properly spelled code, and you as a reviewer should really point out these, if you spot them.
3. Naming – having a properly named class or a method is really something that can help you and your team to make the code as readable and expressive as possible. It is not without reason said that naming things is one of the most difficult things in programming. Sometimes it is very difficult to choose a proper name, so whenever I’m in doubt I ask the author of the code, or reviewer of my own, if we could come up with a better name together. Clear class / method names can help everyone to remember  and reuse them, when needed. Finding a good name can be difficult, but once you figure it, it makes it clear what a class or a method does. This makes it easy for anyone reading the code to understand what’s its usage and how it fits with the rest of the code.
4. Proper usage of existing methods – sometimes, developers are not aware of all the methods that they have at their disposal, and are either creating the similar ones, or using the code that could be replaced with a single method call. I think that it is very important to point out things like this, because whenever you can replace something that you’ve written, with something that does that same thing, already exists and is tested, you should do it.
5. Conventions – whatever conventions we have on a team level, I expect all the team members to pay attention to it and respect it. For example, the whole team should stick to the same coding conventions. Using conventions makes the code uniform and easier to read, and you, as a reviewer, should make sure that the author of the code respects them.
6. Unit tests – whenever I’m doing the code review, I like to check the unit tests. They can show the thought process of the author. Are both happy flow and marginal cases covered? How well are they written, and do they test the right thing? Aiming for the high coverage per se can be very tricky, and I would rather like the developer to cover only the most complex part of the patch, but do it well, rather than to have everything covered poorly.
7. Implementation correctness – you should briefly check if the implementation is correct by comparing it with the feature specification. Sometimes, it’s relatively easy to spot the bugs in the pull requests, if the author has missed to cover an alternative flow. I don’t go into details, but try to figure out what and how the functionality should do, and try to rehearse the implementation in my head, and see if it all fits.
8. Method location – in file – is the method properly positioned in the class file? Are all methods that invoke it above, or there are some above, and some below. This may look as splitting hairs, but it will make the code much more readable if you place it all below. This way, when someone reads a class, he does not have to jump all over it to find it, but can read it in the natural way, from top to the bottom.
9. Method location – in module/project – sometimes, a developer does not pay attention to where the method or piece of code actually belongs. A misplaced piece of code left to sit at the wrong place can cause much deterioration of the code base. Just one simple example would be a misplaced null check, if you decide to ever pass or return nulls from your methods. If you don’t put the null check at the lowest possible level, inside the method which actually does something with the nullable parameter, you will have to check it in many places. As a consequence, this will make it obligatory to always do the null check, will lead to surplus code, and even can cause bugs.
10. Simplicity – is the produced code the simplest possible solution? Sometimes, specially with young developers, the code is unnecessary made very complex, and can be simplified just by using different data structures or method constructs. For example, one might use a list where a map would be a better fit. If you spot an overly complex solution, you should point it out, and maybe even propose a simpler one.
11. Data structures – are proper data structures used to solve the problem? Would it make it more efficient to use some other structure? I think that one of the things that make a good developer is knowing which data structures should be used where, and which are the benefits and drawbacks of using each of them. Special case here is using hash structures, which require that you also have equals() and hashCode() methods overridden for classes you’re storing in them, so please always check if they are present and properly implemented.
12. Resource usage – are any resources that are being used properly handled. Is the file reading stream closed always? Is there a usage of remote systems and does the author use it optimally? Inefficient or improper usage of various resources does not have to create a problem during the development. You might not even notice it there. Still, once your system is under heavy load in production, you can have serious problems because the resources are not released properly, or not used efficiently.
13. Patterns – be it the design or architectural patterns, check if they are properly implemented and in place. They can help a lot to to make the code more readable and maintainable, if used correctly.  Missing to apply one can lead to code which is more complex than needed, the same as overusing it.
14. Optimization – is something prematurely optimized even if it is not a bottleneck? I’m strongly against premature optimizations because mostly they add complexity to the source code. Whenever you see something that looks like it, you should ask why it’s done. Sometimes, the explanation is reasonable, and other times it’s really not needed and you can agree to remove it.
15. SOLID principles – are these principles applied together? Proper application of these principles should make it reasonably easy to maintain and extend your application, so you should check if they have been thought of during the implementation. If you see something that looks like it’s against these principles, it might point out at a code smell, and you should at least discuss it with the author, why it’s done the way it is done.

Here are some additional checks that I do, before I ask somebody else for a review. Although these seem quite obvious, it happens that I forgot to check it, or I see somebody else commit the code without doing it, that I have to add it here.

1. Static code analysis – I certainly hope you have setup a continuos build process, at least, which has a static code analysis running on each build, or at least a nightly. If not, I strongly suggest you use one, because they can point out to potential bugs or security issues in your code. You can also run the static code analysis tools (e.g Checkstyle, FindBugs) locally, and spot the potential issues before you push your code to the master development branch.
2. Check if it all works – this is more a checkpoint for the author, than the reviewer, but I’ve seen it so many times, I have to mention it. After you’ve merged the latest main development branch revision to your feature branch, to make it mergable, did you check if it works? Did you run the build and see if all unit tests pass?

Please note that this list is meant to be used only as a starting point. There are so many other things that you should/might look for in a code review, that whole books are written about it.

Feel free to modify this list however it suits your team, by adding or removing items. Here you can find a downloadable PDF checklist version.

How do you do your code reviews? What are the things important to you? Please feel free to comment, I’d love to hear your opinion.