Check this list before git push!
Every developer knows about the clean code and architecture rules, and we cannot underestimate how important it is to keep the code in the project properly organized, and at the same time readable and maintainable.
The following checklist may be used both when you are about to push your own code to the repository and when you are reviewing the pull/merge request made by your colleague.
Disclaimer: this list is written in the context of the frontend development, as I’m working with the TS+Nuxt/Vue at the moment, so some rules may be not so universal.
Logic and architecture checking:
Is your code simple enough?
This is about the KISS rule, which stands for “keep it simple stupid” — imagine you see your code for the first time, would it be easy for you to understand what each function does, how do they interact with each other, what is each component for? Try to spot any not-so-clear places and consider rewriting them to use a simpler logic.
What are the dependencies between your component and other components, how many other components depend on it?
Avoid logical dependency. Don’t write methods which work correctly only depending on something else in the same class.
Are your components reusable? Try inserting the component in a completely different place, for example, on a different page (of course, passing the proper props/data to it). If you see that your component won’t work as expected even though it has all the required data passed to it in props — it means it’s a weak place in the application architecture. More about this issue you can read in the Chapter 14 of the “Clean Architecture” by Robert C. Martin. The solution here would be to rewrite the logic so that your components are more independent.
Is your solution too difficult to change?
If you see that a small change would cause a cascade of subsequent changes (or can cause errors in many different places) consider rewriting your code. The applications nature is changeable and each solution should be ready to accept the changes and adjust in a more or less simple way, otherwise it won’t survive in the evolution process.
How difficult is it to write a unit test for each function?
Ideally one function should be responsible only for one action. If there is a need to create a more complex function, make sure it is as readable as possible. To check this you can write a simple unit test. You will see if it is easy to predict the function behavior and the result it returns in different cases. Some developers use the test driven approach — they write the tests first, and then they write functions that pass the tests.
Clean code checking:
Are all new names of the variables and functions descriptive and unambiguous?
The names should be not too long, but in the same time as descriptive as possible. If you hesitate if the name you choose is good enough — just ask your colleagues whether they can guess what the function does or a variable may contain when they know only its name.
Are there any magic numbers?
If yes, move them to the constant (or even consider using an enum). This will make your code more readable.
Are there any hardcoded data?
The data that are used in different places in the project, for example, URLs for the API requests, should be stored in one place (‘data’ folder or configuration files, it depends on the project) and imported into components when needed.
How many arguments are passed to the functions?
The good practice is to pass not more than 3 arguments. If you need to pass many different arguments, consider joining them into one object and pass that one object into the function.
Are variables declared close to their usage? Are functions placed in the downward direction?
If you don’t understand why this should be done, consider reading the chapter 4 from the Functional-Light JavaScript. Why functional programming? We know that each programming approach has its pros and cons (more on that subject — see Chapter 3 of the “Clean Architecture” by Robert C. Martin), so in my opinion the good approach is to use the best parts of each in you code.
Are there any duplicates?
For example, functions that are called differently, but actually do the same thing? It’s harder to spot them when they are located in different components. Consider moving them into the helper function (stored in a separate file) and import it into the components where you need to use it.
Are there any redundant comments?
What does each comment describe?
If it describes what the function does — it is redundant, and if without it you cannot understand what the function does — you probably need to rename the function or even rewrite it to make it more clear.
If it divides the parts of the code to “chapters”, consider moving different “chapters” into a separate file (only if it doesn’t make the component code less readable and maintainable).
The good comment should make the code easier to read, but it should not explain what the code does.
An example of a good comment:
// ‘/en/result/’ >> ‘result’
const currentPage = this.$route.path.substr(4).slice(0, -1)An example of a bad comment:
// Save the name into the cookie
document.cookie = `name=${selectedName}`Security and data management
Check for possible leaks of any sensitive data (do you encrypt it before saving in the local/session storage or cookies?). How hard is it to steal the data or insert the fake data instead of the real one? Not all developers think about this, often they are in a rush to deliver their solutions on time, but also not many projects have the proper testing procedure to detect such weak places on the application. It is a developers responsibility to keep sensitive data as secured as possible.
Wash the dishes!
Remove the commented code.
You may want to leave a commented code only in cases when you are sure you will need this functionality in the future.
Remove the console.log and debugger statements.
Search for this statement in the whole project — maybe you or your colleagues forgot to remove it? Even if it has been done by someone else — clean it. Boy scout rule. Leave the campground cleaner than you found it.
Remove unused variables, functions, methods, files.
All of this creates a mess in the project and may be misleading for other developers.
— — — — —
This checklist is of course not complete, we can expand it and add rules which depend on the a specific project and specific technologies and requirements. Happy coding (and checking)!
