Code Review Tips

In the following post, I’ve written an extended list of all the items I check when I read a Merge Request.

I copy the list to the project guideline section and add specific items because there may be additional things to check, specific for the project.

RESTful endpoints

  • Use GET only for reading information, a simple item or a list;
  • Use POST to create an item, or perform an action on the system. Take into account that two consecutive POST won’t have the same result (if that’s not the case, use another verb);
  • Use PUT to update an existing resource, overloading the complete resource;
  • Use PATCH to update some fields of an existing resource;
  • Use DELETE to delete a resource. It can be a hard delete or a soft delete;
  • When working with a single entity, the URL must have the following format /entities (with the plural);
  • When working with an item of an entity, the URL must have the following format /entities/<id>;
  • When there is an action over an entity, the URL must have the following format: /entities/<id>/<action>. The action can be search, import, export… but not update, created or delete.

Naming Convention

  • When using many words, separate them always with the same naming strategy (snake case or camel case);
  • The methods must always contain a verb;
  • When we read a value from an entity, use the verb get in the method. No or few logic can be used in the method;
  • When setting a value to an entity, use the verb set in the method. No or few logic can be used in the method;
  • For other methods, avoid the verbs get and set;
  • Fields must be name in singular, or plural for lists;
  • For maps, name both parts (the key and the value) as entities_per_id;
  • Use the same word for an entity in all the application and documentation. If you use vehicle, use vehicle in the classes, methods and database (not car).

Code Organization

  • When the logic impacts the Web interface, it goes to the controllers;
  • When the logic impacts a single entity, it goes inside the entity itself;
  • When the logic impacts many entities of one type or have many dependencies, create a service dedicated to the entity;
  • When the logic impacts many entities of different types, create a dedicated service;
  • When the logic impacts no entities, create a mapper or parser. Avoid utility classes if possible.

Git

  • The commit message must contain the ticket information;
  • The commit message must contain a short description of the modifications. If a short message is not enough, write a longer description below the title. The more you describe, the best;
  • The commit must contain only one change (not changes of different natures);
  • The commit must contain both the changes and the unit tests;
  • Avoid commit messages like “Fix typo”, “Fix unit test”, “Fix error”.

Conclusion

The key is to share the list with all the development team. This list must be known not only by the one who reads the Merge Requests, but also by the people who create them.


Never Miss Another Tech Innovation

Concrete insights and actionable resources delivered straight to your inbox to boost your developer career.

My New ebook, Best Practices To Create A Backend With Spring Boot 3, is available now.

Best practices to create a backend with Spring Boot 3

Leave a comment

Discover more from The Dev World - Sergio Lema

Subscribe now to keep reading and get access to the full archive.

Continue reading