I was asked to work on this back-end scheduled job that export some customers data (from an e-commerce DB) to a custom-format text file. The code that follows is what I found.
I just would like to delete it all, but I can't. Would it be possible for me to improve this without changing it so much?
public class AConverter implements CustomerConverter {
protected final Logger LOG = LoggerFactory.getLogger(AConverter.class);
private final static String SEPARATOR = ";";
private final static String CR = "\n";
public String create(Customer customer) {
if (customer == null)
return null;
LOG.info("Exporting customer, uidpk: {}, userid: {}", customer.getUidPk(), customer.getUserId());
StringBuilder buf = new StringBuilder();
buf.append("<HEAD>");
buf.append(SEPARATOR);
buf.append(String.valueOf(customer.getUidPk()));
buf.append(SEPARATOR);
byte[] fullName = null;
try {
fullName = customer.getFullName().getBytes("UTF-8");
} catch (UnsupportedEncodingException e1) {
fullName = customer.getFullName().getBytes();
}
String name = null;
try {
name = new String(fullName, "UTF-8");
} catch (UnsupportedEncodingException e) {
name = customer.getFullName();
}
buf.append(limitString(name, 40));
buf.append(SEPARATOR);
final CustomerAddress preferredShippingAddress = customer.getPreferredShippingAddress();
if (preferredShippingAddress != null) {
final String street1 = preferredShippingAddress.getStreet1();
if (street1 != null) {
buf.append(limitString(street1, 40));
}
} else {
buf.append(" ");
}
buf.append(SEPARATOR);
final String addressStr = buildAddressString(customer);
buf.append(limitString(addressStr, 40));
buf.append(SEPARATOR);
buf.append(limitString(customer.getEmail(), 80));
buf.append(SEPARATOR);
if (preferredShippingAddress!=null && preferredShippingAddress.getStreet2() != null) {
buf.append(limitString(preferredShippingAddress.getStreet2(), 40));
} else {
buf.append(" ");
}
buf.append(SEPARATOR);
buf.append(limitString(customer.getPhoneNumber(), 25));
buf.append(SEPARATOR);
if (preferredShippingAddress!=null) {
if(preferredShippingAddress.getCountry()!=null) {
buf.append(preferredShippingAddress.getCountry());
} else {
buf.append(" ");
}
} else {
buf.append(" ");
}
buf.append(SEPARATOR);
if (preferredShippingAddress!=null) {
if(preferredShippingAddress.getCountry()!=null) {
buf.append(preferredShippingAddress.getCountry());
} else {
buf.append(" ");
}
} else {
buf.append(" ");
}
buf.append(SEPARATOR);
String fodselsnummer = " ";
try {
Map<String, AttributeValue> profileValueMap = customer.getProfileValueMap();
AttributeValue attributeValue = profileValueMap.get("CODE");
fodselsnummer = attributeValue.getStringValue();
} catch (Exception e) {
}
buf.append(fodselsnummer);
buf.append(CR);
final String string = buf.toString();
return string;
}
private String buildAddressString(Customer customer) {
final CustomerAddress preferredShippingAddress = customer.getPreferredShippingAddress();
if (preferredShippingAddress != null) {
final String zipOrPostalCode = preferredShippingAddress.getZipOrPostalCode();
final String city = preferredShippingAddress.getCity();
if (zipOrPostalCode != null && city != null) {
return zipOrPostalCode + " " + city;
} else if(zipOrPostalCode == null && city != null) {
return city;
} else if(zipOrPostalCode != null && city == null) {
return zipOrPostalCode;
}
}
return " ";
}
private String limitString(String value, int numOfChars) {
if (value != null && value.length() > numOfChars)
return value.substring(0, numOfChars);
else
return value;
}
}
You say you want to improve it, you'd like to delete it, but you can't. I'm not sure why you can't. I also don't understand why you'd want to delete it. But it sounds to me like the kind of attitude I used to have before I read Refactoring by Martin Fowler. I would strongly suggest you read that book, if you haven't already.
It is certainly possible to improve this code (or any code) without rewriting it all. The most obvious improvements would be to eliminate some of the repetitive code in the
createmethod by creating some utility methods, and then breaking up thecreatemethod into several smaller methods à la template methods.Also, there is a questionable bit of code in the
createmethod that turns the customer's name into a UTF-8 byte stream and then back into a string. I can't imagine what that's for. Finally, it returns null if the customer is null. That is unlikely to be necessary or wise.For fun, I decided to do a little refactoring on this code. (Note that proper refactoring involves unit tests; I don't have any tests for this code and have not even compiled the code below, much less tested it.) Here is one possible way you could rewrite this code:
I also notice that there is a problem with your format for the custom-format text file if any of the fields of the customer data (email address, for example) happen to have a semicolon in them, because that is your separator character. I trust that is a known issue?