SAP Commerce (Hybris) Quick coding exercise - Refactoring the service

You got the code below. You should refactor the code and explain.

BestCarFinder.java:

package com.carsupplier.core.car.services;

public class BestCarFinder {

    private FlexibleSearchService flexibleSearchService;
    private BestsellerFinder bestsellerFinder;

    @Override
    public CarModel getBest(BrandModel brandModel) {
        Map<String, Object> params = new HashMap<String, Object>();
        params.put(CarModel.BRAND, brandModel);
        SearchResult<AbstractOrderModel> results = flexibleSearchService.<CarModel>search(QUERY, params);
        List<CarModel> cars = results.getResult();
        return bestsellerFinder.findBest(cars);
    }

    // other stuff
}

carsuppliercore-spring.xml:

<bean id="LegacyFinder" class="com.carsupplier.core.car.services.BestCarFinder">
    <property name="flexibleSearchService" ref="flexibleSearchService"/>
    <property name="bestsellerFinder" ref="bestsellerFinder"/>
</bean>

1. Wrong design

The class contains business logic. All business logic in hybris should be coded as services. So this is a service and service should be designed as a Spring bean with a Java Interface implemented by a Java class.
What should we do:

• Create a Java interface
• Implement this interface
• Fix the Spring configuration

Let’s create the interface.

package com.carsupplier.core.car.services;

interface CarFinder {
    CarModel getBest(BrandModel brandModel);
}

BestCarFinder should implement it:

package com.carsupplier.core.car.services;

public class BestCarFinder implements CarFinder {

    private FlexibleSearchService flexibleSearchService;
    private BestsellerFinder bestsellerFinder;

    @Override
    public CarModel getBest(BrandModel brandModel) {
        Map<String, Object> params = new HashMap<String, Object>();
        params.put(CarModel.BRAND, brandModel);
        SearchResult<AbstractOrderModel> results = flexibleSearchService.<CarModel>search(QUERY, params);
        List<CarModel> cars = results.getResult();
        return bestsellerFinder.findBest(cars);
    }

    // other stuff
}

And finally fix the spring config:

<alias name="LegacyFinder" alias="carFinder"/>
<bean id="LegacyFinder" class="com.carsupplier.core.car.services.BestCarFinder">
    <property name="flexibleSearchService" ref="flexibleSearchService"/>
    <property name="bestsellerFinder" ref="bestsellerFinder"/>
</bean>

Will that be all? Not really, the solution doesn’t look that great. isn’t it?

2. Wrong naming

The next problem here is incorrect naming. The service interface name should have the “Service” ending at the end. So, we should rename:

• CarFinder to CarFinderService
• BestCarFinder to BestCarFinderService.

Let’s fix the interface name:

package com.carsupplier.core.car.services;

interface CarFinderService {
    CarModel getBest(BrandModel brandModel);
}

Let’s fix the implementation name:

package com.carsupplier.core.car.services;

public class BestCarFinderService implements CarFinderService {

And finally fix the spring config:

<alias name="LegacyFinderService" alias="carFinderService"/>
<bean id="LegacyFinderService" class="com.carsupplier.core.car.services.BestCarFinderService">

And the code still doesn’t look great.

3. Wrong package

The next issue here is an incorrect package of the interface and the implementation, because:

• the interfaces of services should be located in the root package or in a subpackage named after the domain concept if appropriate.
• Package names of services should not contain “services” or “servicelayer”.

So to fix this we have to move the interface and the implementation to the appropriate location:

• the interface from com.carsupplier.core.car.services to com.carsupplier.core.car
• the implementation from com.carsupplier.core.car.services to com.carsupplier.core.car.impl

Fix the interface name:

package com.carsupplier.core.car;

interface CarFinderService {
    CarModel getBest(BrandModel brandModel);
}

Fix the implementation name:

package com.carsupplier.core.car.impl;

public class BestCarFinderService implements CarFinderService {

fix class in the spring config:

<alias name="LegacyFinderService" alias="carFinderService"/>
<bean id="LegacyFinderService" class="com.carsupplier.core.car.impl.BestCarFinderService">

What’s next?

4. Wrong the bean id and the alias name

The next issues here are an incorrect bean id and incorrect speling, because:
- The id should be the same as the implementation class.
- The id and an alias should be starting with a lowercase letter.

So, we have to:

• Rename the bean id LegacyFinderService
• Rename the alias name LegacyFinderService
• The bean id and the alias name should be starting with a lowercase letter

  <alias name="bestCarFinderService" alias="carFinderService"/>
  <bean id="bestCarFinderService" class="com.carsupplier.core.car.impl.BestCarFinderService">

  What else?

5. Encapsulation issue

If you look at the method #getBest you can found that the implemented service contains Flexible search queries inside business logic. This gives us:

• A unclear separation of service business logic from database query logic
• The database query logic can’t be tested separately
• The tricky overriding of a specific database access method

To fix this, create a DAO interface, implement it, and use it as a spring-bean in BestCarFinderService. So the final code should look like this:

package com.carsupplier.core.car;

interface CarFinderService {
    CarModel getBest(BrandModel brandModel);
}

package com.carsupplier.core.car.impl;

public class BestCarFinderService implements CarFinderService {

    private BestsellerFinder bestsellerFinder;
    private CarsDao carsDao;

    @Override
    public CarModel getBest(BrandModel brandModel) {
        List<CarModel> cars = carsDao.findCarsByBrand(brandModel);
        return bestsellerFinder.findBest(cars);
    }

    // other stuff
}

<alias name="bestCarFinderService" alias="carFinderService"/>
<bean id="bestCarFinderService" class="com.carsupplier.core.car.impl.BestCarFinderService">
    <property name="bestsellerFinder" ref="bestsellerFinder"/>
    <property name="carsDao" ref="carsDao"/>
</bean>

Created based on official documentation

Follow me in the group in telegram