Which pattern to use to avoid code duplication with object value transformer

Question

I want to get rid of the following code duplication within the MyFacadeBean. Consider the following situation:

public class FacadeBean implements Facade {

    @EJB
    private CrudService crudService;

    @Inject
    private FirstAssembler firstAssembler;
    @Inject
    private SecondAssembler secondAssembler;
    @Inject
    private ThirdAssembler thridAssembler;
    @Inject
    private FourthAssembler fourthAssembler;

    @Override
    public void save(FirstValue value) {
        FirstEntity entity = this.firstAssembler.transformToEntity(value);
        this.crudService.persist(entity);
    }

    @Override
    public void save(SecondValue value) {
        SecondEntity entity = this.secondAssembler.transformToEntity(value);
        this.crudService.persist(entity);
    }

    @Override
    public void save(ThirdValue value) {
        ThirdEntity entity = this.thirdAssembler.transformToEntity(value);
        this.crudService.persist(entity);
    }

    @Override
    public void save(FourthValue value) {
        FourthEntity entity = this.fourthAssembler.transformToEntity(value);
        this.crudService.persist(entity);
    }

}

public interface MyFacade {

    void save(FirstValue value);

    void save(SecondValue value);

}

With the CrudService:

public interface CrudService {

    void persist(Object entity);

}

@Stateless
@Local(CrudService.class)
@TransactionAttribute(TransactionAttributeType.MANDATORY)
public class CrudServiceBean implements CrudService {

    public static final String PERSISTENCE_UNIT_NAME = "my_persistence_unit";

    private EntityManager entityManager;

    @PersistenceContext(unitName = PERSISTENCE_UNIT_NAME)
    public void setEntityManager(EntityManager entityManager) {
        this.entityManager = entityManager;
    }

    @Override
    public void persist(Object entity) {
        this.entityManager.persist(entity);
    }

}

With the following assemblers:

public class FirstAssembler extends AbstractAssembler<FirstEntity> {

    public FirstEntity transformToEntity(FirstValue value) {
        if (value == null)
            return null;
        FirstEntity entity = new FirstEntity();
        transformAbstractValueToAbstractObject(value, entity);
        entity.setFixedRate(value.getFixedRate());
        entity.setStartDate(value.getStartDate());
        return entity;
    }

}

public class SecondAssembler extends AbstractAssembler<SecondEntity> {

    public SecondEntity transformToEntity(SecondValue value) {
        if (value == null)
            return null;
        SecondEntity entity = new SecondEntity();
        transformAbstractValueToAbstractObject(value, entity);
        entity.setTransactionType(value.getTransactionType());
        entity.setValueDate(value.getValueDate());
        return entity;
    }

}

public abstract class AbstractAssembler<T extends AbstractEntity> {

    protected void transformAbstractValueToAbstractObject(AbstractValue value, T object) {
        object.setUniqueId(value.getUniqueId());
        object.setNominalAmountValue(value.getNominalAmountValue());
    }

}

With the following entities:

@Entity
public class FirstEntity extends AbstractEntity {

    private static final long serialVersionUID = 1L;

    @Id
    @Column(name = "ID")
    private Long id;
    @Column(name = "START_DATE")
    @Temporal(TemporalType.DATE)
    private Date startDate;
    @Column(name = "FIXED_RATE")
    @Digits(integer = 1, fraction = 10)
    private BigDecimal fixedRate;

    public Long getId() {
        return id;
    }

    public void setId(Long id) {
        this.id = id;
    }

    public Date getStartDate() {
        return startDate;
    }

    public void setStartDate(Date startDate) {
        this.startDate = startDate;
    }

    public BigDecimal getFixedRate() {
        return fixedRate;
    }

    public void setFixedRate(BigDecimal fixedRate) {
        this.fixedRate = fixedRate;
    }

}


@Entity
public class SecondEntity extends AbstractEntity {

    private static final long serialVersionUID = 1L;

    @Id
    @Column(name = "ID")
    private Long id;
    @Column(name = "VALUE_DATE")
    @Temporal(TemporalType.DATE)
    private Date valueDate;
    @Column(name = "TRANSACTION_TYPE")
    @Enumerated(EnumType.STRING)
    private TransactionType transactionType;

    public Long getId() {
        return id;
    }

    public void setId(Long id) {
        this.id = id;
    }

    public Date getValueDate() {
        return valueDate;
    }

    public void setValueDate(Date valueDate) {
        this.valueDate = valueDate;
    }

    public TransactionType getTransactionType() {
        return transactionType;
    }

    public void setTransactionType(TransactionType transactionType) {
        this.transactionType = transactionType;
    }

}

@MappedSuperclass
public abstract class AbstractEntity implements Serializable {

    private static final long serialVersionUID = 1L;

    @Column(name = "TRANSACTION_NOM_AMOUNT_VALUE")
    @Digits(integer = 18, fraction = 5)
    @Min(0)
    private BigDecimal nominalAmountValue;

    public BigDecimal getNominalAmountValue() {
        return nominalAmountValue;
    }

    public void setNominalAmountValue(BigDecimal nominalAmountValue) {
        this.nominalAmountValue = nominalAmountValue;
    }

}

I tried the following approach:

public class FacadeBean implements Facade {
    @Inject
    private Assembler assembler;

    @Inject
    private AssemblerFactory assemblerFactory;

    @Override
    public <T extends AbstractValue> void save(T value) {
        Assembler assembler = assemblerFactory.createAssembler(value);
        AbstractEntity entity = assembler.transformToEntity(value);
        this.crudService.persist(entity);
    }
}

Problems are the AssemblerFactoryImpl and the AssemblerImpl in which I have to do instanceOf checks and castings...

Another idea would be to let the value know which transformer to use (or how to transform). But I want the value to be "dumb".

@Glenn Lane

public AbstractValue save(AbstractValue value) {
    AbstractAssembler<AbstractValue, AbstractEntity> assembler = new FirstAssembler();
    AbstractEntity entity = assembler.transformToEntity(value);
    AbstractValue result = assembler.transformToValue(entity);
    return result;
}

does not work, because of

Type mismatch: cannot convert from FirstAssembler to AbstractAssembler

Answer 1

I'm posting this as a separate answer, since I don't really think there's anything wrong with having a save method for every AbstractValue type.

First we'll establish your base value class for this example. I'm using an interface just so we don't muddy the waters. Your AbstractValue interface:

public interface AbstractValue
{
    int getUniqueId();
    double getNominalValue();
    <T> T accept(AbstractValueVisitor<T> visitor);
}

And the "visitor interface":

public interface AbstractValueVisitor<T>
{
    T visit(FirstValue value);
    T visit(SecondValue value);
    T visit(ThirdValue value);
    T visit(FourthValue value);
}

I know you don't want intelligence baked into AbstractValue, but we are going to add one specification... that all concrete implementations of AbstractValue (all four) implement the accept method exactly this way:

@Override
public <T> T accept(AbstractValueVisitor<T> visitor)
{
    return visitor.visit(this);
}

So that method is implemented four times: in all four value classes, exactly the same way. Because the visitor interface is aware of all concrete implementations, the appropriate method will be called for each particular value type. All three of these parts put together is the "visitor pattern".

Now we'll make an entity factory. Its job is to create the appropriate AbstractEntity when provided an AbstractValue:

public class AbstractEntityFactory
        implements AbstractValueVisitor<AbstractEntity>
{
    private static final AbstractEntityFactory INSTANCE;

    static
    {
        INSTANCE = new AbstractEntityFactory();
    }

    // Singleton pattern
    private AbstractEntityFactory()
    {
    }

    public static AbstractEntity create(AbstractValue value)
    {
        if (value == null)
        {
            return null;
        }
        AbstractEntity e = value.accept(INSTANCE);
        e.setNominalValue(value.getNominalValue());
        e.setUniqueId(value.getUniqueId());
        return e;
    }


    @Override
    public AbstractEntity visit(FirstValue value)
    {
        FirstEntity entity = new FirstEntity();
        // Set all properties specific to FirstEntity
        entity.setFixedRate(value.getFixedRate());
        entity.setStartDate(value.getStartDate());
        return entity;
    }

    @Override
    public AbstractEntity visit(SecondValue value)
    {
        SecondEntity entity = new SecondEntity();
        // Set all properties specific to SecondEntity
        entity.setTransactionType(value.getTransactionType());
        entity.setValueDate(value.getValueDate());
        return entity;
    }

    @Override
    public AbstractEntity visit(ThirdValue value)
    {
        ThirdEntity entity = new ThirdEntity();
        // Set all properties specific to ThirdEntity
        return entity;
    }

    @Override
    public AbstractEntity visit(FourthValue value)
    {
        FourthEntity entity = new FourthEntity();
        // Set all properties specific to FourthEntity
        return entity;
    }
}

Now your facade implementation takes an AbstractValue, and you got that one save method you're looking for:

public class FacadeBean implements Facade
{
    @EJB
    private CrudService crudService;

    @Override
    public void save(AbstractValue value)
    {
        AbstractEntity entity = AbstractEntityFactory.create(value);
        crudService.persist(entity);
    }
}

Because your AbstractValue now follows the visitor pattern, you can do all sorts of polymorphic behavior. Such as:

public class AbstractValuePrinter implements AbstractValueVisitor<Void>
{
    private final Appendable out;

    public AbstractValuePrinter(Appendable out)
    {
        this.out = out;
    }

    private void print(String s)
    {
        try
        {
            out.append(s);
            out.append('\n');
        }
        catch (IOException e)
        {
            throw new IllegalStateException(e);
        }
    }

    @Override
    public Void visit(FirstValue value)
    {
        print("I'm a FirstValue!");
        print("Being a FirstValue is groovy!");
        return null;
    }

    @Override
    public Void visit(SecondValue value)
    {
        print("I'm a SecondValue!");
        print("Being a SecondValue is awesome!");
        return null;
    }

    @Override
    public Void visit(ThirdValue value)
    {
        print("I'm a ThirdValue!");
        print("Meh.");
        return null;
    }

    @Override
    public Void visit(FourthValue value)
    {
        print("I'm a ThirdValue!");
        print("Derp.");
        return null;
    }
}

In this example, this visitor isn't returning anything... it's "doing" something, so we'll just set the return value as Void, since it's non-instantiatable. Then you print the value simply:

// (value must not be null)
value.accept(new AbstractValuePrinter(System.out));

Finally, the coolest part of the visitor pattern (in my opinion): you add FifthValue. You add the new method to your visitor interface:

T visit(FifthValue value);

And suddenly, you can't compile. You must address the lack of this handling in two places: AbstractEntityFactory and AbstractValuePrinter. Which is great, because you should consider it in those places. Doing class comparisons (with either instanceof or rinde's solution of a class-to-factory map) is likely to "miss" the new value type, and now you have runtime bugs... especially if you're doing 100 different things with these value types.

Anyhoo, I didn't want to get into this, but there you go :)