Cleaning Code Practically Episode 1

Last Øredev I were at a presentation done by Robert Martin, author of the book Clean Code. I were really impressed by what his message where. So we are trying to follow his word at work and today a topic came up. So a colleague had this code:


public String getRequestString() throws MQException {
String request = "";
request += Helper.leftRightFilled(true, String.valueOf(getSessionNumberAndIncreaseIt()), '0', 3);
request += Helper.leftRightFilled(true, getVersion(), '0', 3);
request += Helper.leftRightFilled(true, getTransaction(), 'X', 4);
request += Helper.leftRightFilled(true, getLanguage(), 'X', 2);
request += Helper.leftRightFilled(true, getCallersPhoneNumber(), '0',16);
request += Helper.leftRightFilled(false, dynamicSettingMQ.getCustomerNumber(), ' ', 16);
request += Helper.leftRightFilled(true, String.valueOf(dynamicSettingMQ.getNumberOfUnsucceededCalls()), '0', 3);
return request;
}


I intermediately though that could be more readable, so I just pushed a lot into functions, and did a little to the helper class:

public String getRequestString() throws MQException {
String request = "";
request += createSessionNumberString();
request += createVersionString();
request += createTransactionString();
request += createLanguageString();
request += createCallersPhoneNumberString();
request += createCustomerNumberString();
request += createNumberOfUnsucceededCallsString();
return request;
}
protected String createCallersPhoneNumberString() throws MQException {
return Helper.leftRightFilled(true, getCallersPhoneNumber(), '0', 16);
}
protected String createCustomerNumberString() throws MQException {
return Helper.leftRightFilled(false, dynamicSettingMQ.getCustomerNumber(), ' ', 16);
}
protected String createLanguageString() throws MQException {
return Helper.leftRightFilled(true, getLanguage(), 'X', 2);
}
protected String createNumberOfUnsucceededCallsString() throws MQException {
return Helper.leftRightFilled(true, dynamicSettingMQ.getNumberOfUnsucceededCalls(), '0', 3);
}
protected String createSessionNumberString() throws MQException {
return Helper.leftRightFilled(true, getSessionNumberAndIncreaseIt(),'0', 3);
}
protected String createTransactionString() throws MQException {
return Helper.leftRightFilled(true, getTransaction(), 'X', 4);
}
protected String createVersionString() throws MQException {
return Helper.leftRightFilled(true, getVersion(), '0', 3);
}
}


At first the colleague thought it was nice, but then remembered he really needed a overview of locations used in the string manipulation. So a thing between the two are needed:

public String getRequestString() throws MQException {
String request = "";
request += createSessionNumberString('0', 3);
request += createVersionString('0', 3);
request += createTransactionString('X', 4);
request += createLanguageString('X', 2);
request += createCallersPhoneNumberString('0', 16);
request += createCustomerNumberString(' ', 16);
request += createNumberOfUnsucceededCallsString('0', 3);
return request;
}
protected String createCallersPhoneNumberString(char fillWith, int length) throws MQException {
return Helper.leftRightFilled(true, getCallersPhoneNumber(), fillWith,length);
}
protected String createCustomerNumberString(char fillWith, int length) throws MQException {
return Helper.leftRightFilled(false, dynamicSettingMQ.getCustomerNumber(), fillWith,length);
}
protected String createLanguageString(char fillWith, int length) throws MQException {
return Helper.leftRightFilled(true, getLanguage(), fillWith,length);
}
protected String createNumberOfUnsucceededCallsString(char fillWith, int length) throws MQException {
return Helper.leftRightFilled(true, dynamicSettingMQ.getNumberOfUnsucceededCalls(), fillWith,length);
}
protected String createSessionNumberString(char fillWith, int length) throws MQException {
return Helper.leftRightFilled(true, getSessionNumberAndIncreaseIt(),fillWith,length);
}
protected String createTransactionString(char fillWith, int length) throws MQException {
return Helper.leftRightFilled(true, getTransaction(), fillWith,length);
}
protected String createVersionString(char fillWith, int length) throws MQException {
return Helper.leftRightFilled(true, getVersion(), fillWith, length);
}


Im not satisfied, but not sure it can be done more clean? And if you are wondering about why we are doing all this string manipulation, we are working with some legacy MQ. If we split up the code without parameters as above we lose the quick overview.. But could the code be simplified even more please come with your suggestion..