DEV Community

Aleksandr
Aleksandr

Posted on

SAP Hybris code review checklist

Alt Text

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);
Enter fullscreen mode Exit fullscreen mode
  • 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

Discussion (0)