SAP Hybris code review checklist

General

Because the code should be fully extensible and easy to test, the reviewed code should not contain:

• final methods/classes (except low-level platform features)
• private methods
• static methods
• Utility classes should have only static methods and with very simple functionality.

*-items.xml

• All Item Types which are extending GenericItem should have a deployment table.
• All many-to-many relations should have a deployment table.
• Deployment type codes must be greater than 10000.
• Check that more abstract types are defined more to the beginning of the items.xml and more concrete types are more to the end.
• Item type names must be started with an uppercase letter.
• Item type names should not be started with Generated
• Attribute names should be started with a lowercase letter.
• Item type must have at least one unique attribute.
• The Item type assignment must be described in the description tag.
• Mandatory attributes (optional="false") should have either initial value defined.
• Boolean attributes must be defined (because they could store null/true/false).
• Relations that have cardinality='many' should not have option ordered='true' unless absolutely necessary.
• CatalogVersion attributes must be unique for Catalog aware types.
• Unique attributes should have database indexes.
• Check whether the attribute needs to be localized.
• Check those attribute definitions are not changed.
• Check that deployment code or table names are not changed.

ImpEx

• There is no mixing headers
• There is INSERT_UPDATE rather than INSERT in Large, One-hit Imports
• Many-to-many Relations are imported separately
• Essential data impexes should only contain must needed data

Service and DAO

• Name of the interfaces of services should be the related domain concept with Service on the end.
• The interfaces of services should be located in the root package or in a subpackage named after the domain concept if appropriate (example: de.hybris.platform.catalog.CatalogService)
• Services should not contain Flexible search queries inside business logic
• The names of interfaces for DAO should be ended with DAO
• DAO interfaces should be located in a package daos underneath the package of the service interface. (example: de.hybris.platform.catalog.daos.CatalogDao, de.hybris.platform.catalog.daos.impl.DefaultCatalogDao)
• DAO methods should be named as findByXXXX where XXXX lists the parameters for the method
• DAO/Service should not return null
• DAO should return an empty list if nothing is found
• DAO should use FlexibleSearchService.searchUnique() when searching for a single item
• DAO should use pagination for potentially large queries to prevent performance issues
• Services method should be named with the following pattern: getFor(). For example:

ProductModel getProductForCode(String code);
List<ProductModel> getProductsForCategory(CategoryModel category);
List<ProductModel> getOnlineProductsForBrand(BrandModel brand);

• If more than one is found for a single result, it should throw AmbiguousIdentifierException
• If none is found, it should throws UnknownIdentifierException
• It is useful to use a parameter object instead of a long list of method parameters.
• Instead of IllegalArgumentException use ServicesUtil.validateParameterNotNull,or analogs to check if an input parameter was provided.

Testing

• Each class should have a related test class (including web classes) in the same package as the class it is testing but under the testsrc folder.
• All public methods should be covered by unit tests.
• Happy path scenario should be covered
• All exception scenarios should be covered
• Basic edge cases should be covered
• Tests should not depend on other tests
• Tests should roll back their transaction or clean up after themselves completely
• Tests should be responsible for inserting the data they require

Misc

• Existed properties should not be changed
• Properties should be documented
• Listener classes should not contain any significant business logic inside.
• DTO beans should be defined in beans.xml files instead of java classes.
• Controllers should use Facades, not Services and Daos

Created based on official documentation

Follow me on my tech blog and telegram