What is the "proper" way to write this in Java?

I have a class that has 5 similar methods, they just relay input parameters and return a result - best practice would be to skip the class, but that's not what I'm asking about :) This is a method from the class:

public String deleteItem(Integer itemId,String username, String kpNumber) {
try {
} catch (RemoteException re) {
String message = "5:unexpected error:" + re.toString();
logger.error(message);
return message;
} catch (CoreException ce) {
String message = "5:unexpected error:" + ce.toString();
logger.error(message);
return message;
}
}


Now, since all the methods are similar, each has the same catch block, each does the same things... so I'd like to rewrite this but I'm unsure how far should I go in "atomizing" the methods. One way is what I consider the "pure" way, so the new code looks like this:

public String deleteItem(Integer itemId,String username, String kpNumber) {
try {
} catch (RemoteException re) {
logException(re);
return getMessage(re);
} catch (CoreException ce) {
logException(ce);
return getMessage(re);
}
}

private String getMessage(Exception exception){
return "5:unexpected error:" + exception.toString();
}

private void logException(Exception exception){
logger.error(getMessage(exception));
}


but I'm also left wondering if I should combine these two methods into one method, like this

public String handleException (Exception e){
String message = "5:unexpected error:" + exception.toString();
logger.error(message);
return message;
}


so what I guess I'm trying to ask is how far should you go in separation of concerns when you have clear and simple cases like this one.

Edit: It seems I have not managed to make myself clear, so I'll try again :) In this case simple case, is it considered better to write one method:

public String handleException (Exception e){
String message = "5:unexpected error:" + exception.toString();
logger.error(message);
return message;
}


or should I have two:

private String getMessage(Exception exception){
return "5:unexpected error:" + exception.toString();
}

private void logException(Exception exception){
logger.error(getMessage(exception));
}

• In what way are those methods different? Jun 19 '12 at 13:07
• In this two lines public String deleteItem(Integer itemId,String username, String kpNumber) and return getItemServiceFacade().deleteItem(itemId, username, kpNumber); Jun 19 '12 at 13:28

One alternative, although I don't know if you think it's too complex, would be to apply a visitor pattern:

private interface Command {
}

public String deleteItem(final Integer itemId, final String username, final String kpNumber) {
return execute(new Command() {
@Override
}
});
}

// other methods...

private String execute(Command command) {
try {
} catch (RemoteException re) {
String message = "5:unexpected error:" + re.toString();
logger.error(message);
return message;
} catch (CoreException ce) {
String message = "5:unexpected error:" + ce.toString();
logger.error(message);
return message;
}
}


I used this pattern when faced with a similar though slightly more complicated situation (implementing optimistic locking for a few different operations).

• This is a bit too complex, but the thing is, I was asking about optimizing the part in the Catch block, not making the method call generic :) Plus, I'm working on Java 1.4 :) Jun 19 '12 at 13:53

There are several things you might want to think about.

The method returns a string. I'm not sure what the facade might return, but to me a boolean or the id (-1 if the method invocation fails) would make more sense. Return the string created by the exceptions requires you not only to implement the try {} catch {} finally block for the checked exceptions, but also forces you to check for an error when the method has been invoked. Since that is what you are returning to the system.

I guess i would think of something like

public boolean deleteItem(Integer itemId,String username, String kpNumber) {
try {