Code duplication when converting DTO to String messages

Question

In a payment application a day before and at the same time with some payments some messages(email) needs to be sent. I have a DTO (called EscrowPayment, projected from some entity) from which I generate and send two similar `messages.

I look for advice specifically on the following issues with the code I ended up:

• Copy-pasted code.
• Type suffix in identifiers. eg: nominalAmountStr
• Static utility methods formatInteger etc (Used application wide).
• String constants in code. But because they are parameterized by position and not by name; if I extract them to a configuration file as they are, I wouldn't know in which order I should be giving parameters to String.format.

Here is the code; it is sanitized and translated, but issues are clearly identifiable:

public class EscrowPaymentMessageServiceImpl {
 private JavaMailSender mailSender;
 // .....
 @Override
 public void escrowPaymentWillBeDone(EscrowPayment escrowPayment) throws BusinessLayerException {
 try {
 String nominalAmountStr = formatInteger(new BigDecimal(escrowPayment.getNominal()));
 String paymentDateStr = dateFormat(escrowPayment.getPaymentDate(), "dd/MM/yyyy");
 String interestRateStr = format(escrowPayment.getInterestRate(), "0.00###");
 String paymentAmountStr = format(escrowPayment.getPaymentAmount());
 String messageText = String
 .format("BLAH BLAH some security with ID %s of nominal value EUR %s "
 + "at date %s BLAH your account with number 1234567890 will be debited to pay "
 + "EUR %s coupon payment for %%%s interest.", 
 escrowPayment.getSecurityId(), nominalStr, paymentDateStr, 
 interestRateStr, paymentAmountStr);
 sendMessage(messageText);
 } catch (MessagingException e) {
 getLog().error("An error occured while sending the email:", e);
 throw new BusinessLayerException("EscrowPaymentMessage.MessageCouldNotBeSent", e);
 }
 }
 @Override
 public void escrowPaymentIsDone(EscrowPayment escrowPayment) throws BusinessLayerException {
 try {
 String nominalAmountStr = formatInteger(new BigDecimal(escrowPayment.getNominal()));
 String paymentDateStr = dateFormat(escrowPayment.getPaymentDate(), "dd/MM/yyyy");
 String paymentAmountStr = format(escrowPayment.getPaymentAmount());
 String messageText = String.format("BLAH BLAH at date %s the security with ID %s of nominal value EUR %s "
 + "BLAH your account with number 1234567890 has been debited to pay EUR %s." ,
 paymentDateStr, escrowPayment.getSecurityId(), nominalStr, paymentAmountStr);
 sendMessage(messageText);
 } catch (MessagingException e) {
 getLog().error("An error occured while sending the email:", e);
 throw new BusinessLayerException("EscrowPaymentMessage.MessageCouldNotBeSent", e);
 }
 }
 private void sendMessage(messageText) throws MessagingException {
 //....
 }
}

• Bobby: There's not much I can see, is using a template system like StringTemplate an option?
• abuzittin gillifirca: Thanks @Bobby. I checking it out, but introducing a new dependency would be a little too costly for me in this occasion.

Answer 1

This strikes me as an occasion where a custom method with positional format indexing may help (Formatter argument_index). It is a little-known feature of Java's String.format / Formatter class, that you can reference values by their position. Consider the following methods:

@Override
public static final String formatEscrowPayment(String format, EscrowPayment escrowPayment) {
 String nominalAmountStr = formatInteger(new BigDecimal(escrowPayment.getNominal()));
 String paymentDateStr = dateFormat(escrowPayment.getPaymentDate(), "dd/MM/yyyy");
 String interestRateStr = format(escrowPayment.getInterestRate(), "0.00###");
 String paymentAmountStr = format(escrowPayment.getPaymentAmount());
 return String.format(format, 
 escrowPayment.getSecurityId(), // 1
 nominalStr, // 2
 paymentDateStr, // 3
 interestRateStr, // 4
 paymentAmountStr); // 5
}
private static final void sendExcrowMessage(String template, ExcrowPayment escrow) throws BusinessLayerException {
 try {
 String messageText = formatEscrowPayment(template, escrow);
 sendMessage(messageText);
 } catch (MessagingException e) {
 getLog().error("An error occured while sending the email:", e);
 throw new BusinessLayerException("EscrowPaymentMessage.MessageCouldNotBeSent", e);
 }
}
@Override
public void escrowPaymentWillBeDone(EscrowPayment escrowPayment) throws BusinessLayerException {
 sendEscrowMessage("BLAH BLAH some security with ID %1$s of nominal value EUR %2$s "
 + "at date %3$s BLAH your account with number 1234567890 will be debited to pay "
 + "EUR %5$s coupon payment for %%%4$s interest.",
 escrowPayment);
@Override
public void escrowPaymentIsDone(EscrowPayment escrowPayment) throws BusinessLayerException {
 sendEscrowMessage("BLAH BLAH at date %3$s the security with ID %1$s of nominal value EUR %2$s "
 + "BLAH your account with number 1234567890 has been debited to pay EUR %2$s." ,
}

Note how the substitution values are handled by their positions.... the %1$s references the first format value as a string. The %3$s references the 3rd value. You can change the order of the positional fields without having to change the positions of the values in the actual format call. Have a look at the documentation

• abuzittin gillifirca: Thanks. This is what was I trying to do in the first place. But I forgot about argument indexes in format strings, and forgot that I forgot. But this is what SE is for. I'll wait for another day or two before accepting, in case someone comes up with another answer that I don't know that I don't know :).