PHP Code review checklist
Checklist to assist with the code reviews.
I love to do code reviews because it gives me chance to see how other people write code and improve mine also. I have seen many people who are afraid of doing code review. Which made think of creating a code review checklist for PHP. Please note this is not a full checklist for code review and following all the conditions in this will not end up in a great code. But following this will end up in code that can be maintained by others in the later stage of code development.
- No syntax/runtime errors and warnings in the code.
- No deprecated functions in the code.
- There should be an explanation for any code that is commented out. “Dead Code” should be removed. If it is a temporary hack, it should be identified as such.
- Check if code has file/class/function level comments. And each of this comments should explain what the file/class/function is doing inside it.
- Check that each function is doing only a single thing. That is a function named createCustomer should never delete the existing customer and create it again.
- A method should not be larger than 40 lines of code. The idea is, the function should be always viewable on a single screen without scrolling.
- Always try to initialize the variable before using that in a function.
- No public class attributes.
- Always try to use constants in the left-hand side of the comparison. That is instead of doing if ($variable == 2) always use if (2 == $variable) because this will help to identify the errors in the earlier stage of development even if we miss and “=” from that statement.
- Never ever mix the PHP code and template (view). In an ideal condition, a view should not contain any logic.
- Always try using single quote ( ‘ ) when working with the PHP string because it has a minor performance improvement when compared to the double quotes ( ” ). The difference comes from the factor that ( ” ) can be used to build a string with variables inside it.
- There should be no catch blocks which catches an exception and throw that again. This is because the exception can bubble up to the top function call stack automatically.
- Never ever throw a general exception with a custom message. Always try to create a custom exception so that all the other code can handle this situation correctly.
- Make sure that the top most code handles all the exceptions correctly shows correct / understandable error messages to the user.
- In the case of a system crash never ever put up the error information that exposes the internal behaviour of the system.
- Make sure that a proper and uniform coding standard is followed throughout the files. I have seen many projects where people mix up the coding style. For PHP I usually use PHP Code Sniffer for validating the coding style.
- Try using the tools like PMD to identify possible bug, dead code, overcomplicated expression, suboptimal code, duplicate code, etc.
- No magic numbers. There should be no magic numbers like 6,10 etc… any numbers like this should be defined as a constant. And Constants should be well commented about the purpose.
- Never allow bad code with some good comments.
- Always try to have a unit test for the new piece of code. In an ideal condition, we should have 100% unit test coverage.
- Never allow a unit test that is written to show 100% coverage and doesn’t do anything that unit test is supposed to do. I have seen unit test code which just calls the function to get 100% coverage and doesn’t have any assertions.
- Always commit /Rollback database transaction at the earliest. Keep the database transaction short as possible.
- Always have an eye on the recursive functions.
- Optimisations may often make code harder to read and more likely to contain bugs. Such optimisations should be avoided unless a need has been identified. Has the code been profiled? Check if any over optimisation has led to functionality disappearing.
- As a reviewer, you should understand the code. If you don’t, the review may not be complete, or the code may not be well commented.
- Lastly, when executed, the code should do what it is supposed to.