Avoid DRY in @AfterMapping-CodePudding

I have two MapStruct Mapping classes for the same Bird entity, with Methods (customCodeBirdMapping) that do almost the same thing but have slightly different return types.

BirdViewMapper

@Mapper(componentModel = "spring")
public interface BirdViewMapper {


    @Mapping(source = "bird.listHealthCheck", target = "listHealthCheck")
    BirdViewDTO birdToBirdViewDTO(Bird bird);

    @AfterMapping
    default void customCodeBirdMapping(Bird source, @MappingTarget BirdViewDTO target) {

        if (!source.getListTransmitter().isEmpty()) {
            Transmitter trans = (source.getListTransmitter()
                    .stream()
                    .max(Comparator.comparing(Transmitter::getDateAttached)).get());
            target.setChannel(trans.getChannel());
            target.setChannelOffset(trans.getChannelOffset());
        }
        if (!source.getListPIT().isEmpty()) {
            PIT pit = (source.getListPIT()
                    .stream()
                    .max(Comparator.comparing(PIT::getDateInserted)).get());
            target.setPitCode(pit.getCode());
        }
        if (!source.getListHealthCheck().isEmpty()) {
            HealthCheck healthCheck = (source.getListHealthCheck()
                    .stream()
                    .max(Comparator.comparing(HealthCheck::getCatchDate)).get());
            target.setDateLastHealthCheck(healthCheck.getCatchDate());
        }

    }

}

BirdMapper

@Mapper(componentModel = "spring")
public interface BirdMapper {

    BirdDashboardDTO birdToBirdListDTO(Bird bird);

    @AfterMapping
    default void customCodeBirdMapping(Bird source, @MappingTarget BirdDashboardDTO target) {

        if (!source.getListTransmitter().isEmpty()) {
            Transmitter trans = (source.getListTransmitter()
                    .stream()
                    .max(Comparator.comparing(Transmitter::getDateAttached)).get());
            target.setChannel(trans.getChannel());
            target.setChannelOffset(trans.getChannelOffset());
        }
        if (!source.getListPIT().isEmpty()) {
            PIT pit = (source.getListPIT()
                    .stream()
                    .max(Comparator.comparing(PIT::getDateInserted)).get());
            target.setPitCode(pit.getCode());
        }
        if (!source.getListHealthCheck().isEmpty()) {
            HealthCheck healthCheck = (source.getListHealthCheck()
                    .stream()
                    .max(Comparator.comparing(HealthCheck::getCatchDate)).get());
            target.setDateLastHealthCheck(healthCheck.getCatchDate());
        }

    }
}

Notice that both BirdViewMapper and BirdMapper use an @AfterMapping to set the most recent value of some child entities but the return/target types are different.

The Bird DTOs are for different views, one for a list of birds (only a few fields) one for a view of a single bird many fields.

I am breaking the DRY rule with the customCodeBirdMapping method on my mapper classes - is there a better way - a way to avoid the repetition?

CodePudding user response：

In both of your examples you show that you have a Mapper for a BirdDTO of some sorts. There are multiple approaches to your Problem.

Merge both mappers into one. They are related anyways. Then simply write usual java private methods in your mapper. These can be called in you @AfterMapping method. This way you prevent code duplications. The easiest way would be obviously to have the DTOs inheriting values.

@Data
public class BirdDTOParent{
    private Channel channel;
    // fields shared by BirdViewDTO and BirdDashboardDTO
}

@Data
public class BirdViewDTO exntends BirdDTOParent{
    // specific fields to BirdViewDTO 
}

@Data
public class BirdDashboardDTO extends BirdDTOParent{
    // specific fields to BirdDashboardDTO  
}

And the method in you mapper:

@AfterMapping
default void customCodeBirdMapping(Bird source, @MappingTarget BirdDashboardDTO target) {
   mappingHelper(source,target);
}

private void mappingHelper(Bird source, BirdDTOParent target) {
  if (!source.getListTransmitter().isEmpty()) {
        Transmitter trans = (source.getListTransmitter()
                .stream()
                .max(Comparator.comparing(Transmitter::getDateAttached)).get());
        target.setChannel(trans.getChannel());
        target.setChannelOffset(trans.getChannelOffset());
    }
    ...
}

If you dont want to do this then you can always cast during runtime. This is however much uglier. If the DTOs have so much in commen you should let the inherit the values

private void mappingHelper(Bird source, Object target) {
    if (!source.getListTransmitter().isEmpty()) {
        Transmitter trans = (source.getListTransmitter()
                .stream()
                .max(Comparator.comparing(Transmitter::getDateAttached)).get());
    if(target instanceof BirdDashboardDTO ) {
        ((BirdDashboardDTO) target).setChannel(trans.getChannel());
        ((BirdDashboardDTO) target).setChannelOffset(trans.getChannelOffset()); 
    } else if (target instanceof BirdViewDTO) {
        ((BirdViewDTO) target).setChannel(trans.getChannel());
        ((BirdViewDTO) target).setChannelOffset(trans.getChannelOffset());
    }
    ...
}

You can have Mappers use the logic of other mappers.
```
@Mapper(componentModel = "spring", uses = {SomeOtherMapper.class})
public interface BirdMapper {
```
even functions you define with @Named annotations can then be used accross both the original and the "using" mapper
Use a static helper class or spring bean. Mapstruct allows you to have spring beans injected into your mappers. You have to changed your Mapper from an interface to an abstract class to achieve this Mapstruct - How can I inject a spring dependency in the Generated Mapper class.

I think the cleanest approach is the first one. But I guess that is up to you :)

EDIT

The first approach can be further refined by creating a @AfterMapping Method for the Parent class and ad @AfterMapping for each child implementation

@Mapper(config = MappingConfig.class)
public interface BirdMapper {


    BirdViewDTO entityToViewDTO(BirdEntity birdEntity);

    BirdDashboardDTO entityToDashboardDTO(BirdEntity birdEntity);

    @AfterMapping
    default void mapParent(BirdEntity source, @MappingTarget BirdDTOParent target){
        System.out.println("aaaaa");
    }

    @AfterMapping
    default void mapViewDTO(BirdEntity source, @MappingTarget BirdViewDTO target){
        System.out.println("bbbbbb");
    }

    @AfterMapping
    default void mapDashboardDTO(BirdEntity source, @MappingTarget BirdDashboardDTO target){
        System.out.println("cccccc");
    }
}

The generated mapping file then looks like this

@Generated(
value = "... mapstruct generated string ...",
date = "... mapstruct generated string ...",
comments = "... mapstruct generated string ...")
@Component
public class BirdMapperImpl implements BirdMapper {

    @Override
    public BirdViewDTO entityToViewDTO(BirdEntity birdEntity) {
        if ( birdEntity == null ) {
            return null;
        }

        BirdViewDTO birdViewDTO = new BirdViewDTO();

        birdViewDTO.setName( birdEntity.getName() );
        birdViewDTO.setViewProperty( birdEntity.getViewProperty() );

        mapParent( birdEntity, birdViewDTO );
        mapViewDTO( birdEntity, birdViewDTO );

        return birdViewDTO;
    }

    @Override
    public BirdDashboardDTO entityToDashboardDTO(BirdEntity birdEntity) {
        if ( birdEntity == null ) {
            return null;
        }

        BirdDashboardDTO birdDashboardDTO = new BirdDashboardDTO();

        birdDashboardDTO.setName( birdEntity.getName() );
        birdDashboardDTO.setDashboardProperty( birdEntity.getDashboardProperty() );

        mapParent( birdEntity, birdDashboardDTO );
        mapDashboardDTO( birdEntity, birdDashboardDTO );

        return birdDashboardDTO;
    }
}

CodePudding user response：

Perspective

For perspective, it may be worth watching Dan Abramov's talk about the WET codebase.

To DRY or not to DRY

When considering a refactor to deduplicate (DRY up) some code, one should ask themselves if the relevant code sections are coincidentally the same, or if they are necessarily the same. In your case, do your two bird classes share the same code by definition? If they don't, a small change may cause the two pieces of code to diverge, and if they have been merged/deduplicated/DRYed up then the DRY code ends up becoming monolithic, or must be split back out regardless. The two bird classes do appear to be the same by definition in your case, and thus are prime candidates for deduplication. In most cases, however, the two similar pieces of code are not exactly the same and the answer is not so clear-cut.

How to DRY

When deduplicating code, be wary, as a class (or OOP in general) is a tool that can easily make everything look like a nail (to pervert the old adage). In this case it may be tempting to introduce a common superclass, but a static utility function is likely more flexible and appropriate here.