Avoid DRY in @AfterMapping

Question

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?

Do the Bird DTOs share some common parent? Where the lists are stored? — Filip, Apr 16 '22 at 06:10

GJohannes · Answer 1 · 2022-04-16T17:31:12.117

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;
    }
}

Thanks, In your first method, wouldnt you have two aftermappings? One for each of the DTOs, but then calling mapping Helper? — Al Grant, Apr 16 '22 at 16:30
Well yes. If you want to save even this call this can also be done by creating a @AfterMapping for each DTO and a AfterMapping for the ParentDTO. I have added an example to my answer that works on my machine ;). — GJohannes, Apr 16 '22 at 17:33

score 0 · Answer 2 · answered Apr 17 '22 at 01:57

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.

Note I took a very philosophical approach to answering your question "I am breaking the DRY rule". If you are in fact seeking more philosophical answers, you may be better served posting questions like this on [Software Engineering StackExchange](https://softwareengineering.stackexchange.com/). If you wanted a more concrete answer, apologies in advance, you'll likely find the other answers here serve you better. — Codebling, Apr 17 '22 at 01:59

Avoid DRY in @AfterMapping

2 Answers2

Perspective

To DRY or not to DRY

How to DRY