This has been (and will be again) written and explained in length by talented writers, but I thought having a very short list can help.
For example you can modify this list as you prefer and share it with junior devs you are about to review, so they can "get there" faster.
The short list
The 4 aspects below are on top of my list for any review:
- small functions doing a single task
- no "copy-paste" code (DRY)
- good naming of functions and variables
- separation of concerns
Some Details
The explanations below are more my own "gut feeling"; people can discuss or disagree endlessly about some of these points. Feel free to adapt/modify according to your own opinion.
Good naming of variables
No need for a comment to understand what a variable contains - or at least easy to remember when you read its comment once.
Good naming of functions
No comment should be needed next to a function call to understand what calling this function does.
Comments in the function's code can explain HOW it is done, or WHY it is done this way, of course!
Separation of concerns
Code is where it should be, data and functions are private as much as possible, etc.
Note the "etc.": there is a lot to say/explain about how this can be done, and usually too much for a code review.
Also, this is often about system design, hence the code review might happen too late to change things.
Finally, it is the hardest concept to explain to junior developers.
For these reasons, this point is last in my short list. This is a bit like a "Trojan horse" bullet point, with an army of concepts hidden within.
Top comments (0)