patternjavaMinor
What is the "proper" way to write this in Java?
Viewed 0 times
thisthewhatwaywritejavaproper
Problem
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:
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:
but I'm also left wondering if I should combine these two methods into one method, like this
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
public String deleteItem(Integer itemId,String username, String kpNumber) {
try {
return getItemServiceFacade().deleteItem(itemId, username, kpNumber);
} 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 {
return getItemServiceFacade().deleteItem(itemId, username, kpNumber);
} 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
Solution
One alternative, although I don't know if you think it's too complex, would be to apply a visitor pattern:
I used this pattern when faced with a similar though slightly more complicated situation (implementing optimistic locking for a few different operations).
private interface Command {
String apply(ItemServiceFacade facade) throws RemoteException, CoreException;
}
public String deleteItem(final Integer itemId, final String username, final String kpNumber) {
return execute(new Command() {
@Override
public String apply(ItemServiceFacade facade) throws RemoteException, CoreException {
return facade.deleteItem(itemId, username, kpNumber);
}
});
}
// other methods...
private String execute(Command command) {
try {
return command.apply(getItemServiceFacade());
} 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).
Code Snippets
private interface Command {
String apply(ItemServiceFacade facade) throws RemoteException, CoreException;
}
public String deleteItem(final Integer itemId, final String username, final String kpNumber) {
return execute(new Command() {
@Override
public String apply(ItemServiceFacade facade) throws RemoteException, CoreException {
return facade.deleteItem(itemId, username, kpNumber);
}
});
}
// other methods...
private String execute(Command command) {
try {
return command.apply(getItemServiceFacade());
} 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;
}
}Context
StackExchange Code Review Q#12728, answer score: 3
Revisions (0)
No revisions yet.