SAP Hybris code review checklist
Tags: hybris, codereview, java
Published: 2021-07-10 13:30:21

![Alt Text](https://dev-to-uploads.s3.amazonaws.com/uploads/articles/gdm9wfvh0w642d1isk3s.jpg)

## 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: get<Something>For<Parameters>(). For example:
```java
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


<em>Created based on official documentation</em>

Follow me on [my tech blog](http://enterprise-developer.com/) and [telegram](https://t.me/enterprise_developer)