less String intensive
I am trying to make the following code less String intensive, I have made certain changes like returning values directly rather than first storing them in local variables. But I am not sure if thats the right way. Any input would be welcome. By it less String intensive the idea is to make it less memory intensive.
private String getInstanceAttributeVale(String attributeName, Insurance ins)throws ItemNotExistException, InvalidFieldException,
InvalidDataTypeException{
String agentName = ins.getAgentName();
String insuranceName = ins.InsName();
String productName = productLookup.findByExternalSrcSys(ins.getProductSystem());
if (null != getStringFromMem(attributeName, insuranceName, agentName, productName)){
return getStringFromMem(attributeName, insuranceName, agentName, productName);
}elseif (null != getStringFromMem(attributeName, INS_NAME_DEFAULT, AGENT_NAME_DEFAULT, productName)){
return getStringFromMem(attributeName, INS_NAME_DEFAULT, AGENT_NAME_DEFAULT, productName);
}elseif (null != getStringFromMem(attributeName, insuranceName, agentName,
NameConstants.SYSTEM_DEFAULT_PRODUCT)){
return getStringFromMem(attributeName, insuranceName, agentName,
NameConstants.SYSTEM_DEFAULT_PRODUCT);
}else{
return getStringFromMem(attributeName, INS_NAME_DEFAULT, AGENT_NAME_DEFAULT,
NameConstants.SYSTEM_DEFAULT_PRODUCT);
}
}
> I can't see anything which is string intensive there,
> but it depends on what getStringFromMem does.
>
> Your code does however have an odd/bad design.
> Creating variables does not create new strings. You
> create multiple calls to getStringFromMem when you
> don't need to.
>
> Kaj
Agreed, that's the only thing I could see there. Use some more temp Strings. nothing wrong with having 10 String objects if you need them. It's probably better than calling that get multiple times with the same parameters.
Agreed. But to make it more readable, you can try this:
if (null != getStringFromMem(attributeName, insuranceName, agentName,
productName)) {
return getStringFromMem(attributeName, insuranceName, agentName,
productName);
}
if (null != getStringFromMem(attributeName, INS_NAME_DEFAULT,
AGENT_NAME_DEFAULT, productName)) {
return getStringFromMem(attributeName, INS_NAME_DEFAULT,
AGENT_NAME_DEFAULT, productName);
}
if (null != getStringFromMem(attributeName, insuranceName, agentName,
NameConstants.SYSTEM_DEFAULT_PRODUCT)) {
return getStringFromMem(attributeName, insuranceName, agentName,
NameConstants.SYSTEM_DEFAULT_PRODUCT);
}
return getStringFromMem(attributeName, INS_NAME_DEFAULT,
AGENT_NAME_DEFAULT, NameConstants.SYSTEM_DEFAULT_PRODUCT);
}
> Agreed. But to make it more readable, you can try
> this:
>
> > if (null != getStringFromMem(attributeName,
> insuranceName, agentName,
> productName)) {
> return getStringFromMem(attributeName,
> me, insuranceName, agentName,
> productName);
> }
>
I would write that as:
String tmp = getStringFromMem(attributeName, insuranceName, agentName, productName));
if (tmp != null) {
return tmp;
}
That does not create extra strings, and it performs better than two calls to getStringFromMem
Kaj
