I have recently inherited some code that I need to maintain that attempts to read information from another machine. That other machine may occasionally become unresponsive long enough for the class I'm working with to time out. However, typically, if I invoke the method that times out a second time, it will work the second time (the other machine eventually responds).
So I need to implement some retry logic to discriminate between the times when the other machine is truly unresponsive and hung up somehow, and when it is just slow in responding.
I can easily write code that gets the job done:
final int MAX_RETRYS = 3;
int accumulatedRetrys = 0;
while (true) {
try {
foo();
break;
} catch (FooException e) {
if (++accumulatedRetrys >= MAX_RETRYS) {
throw new BarException();
}
}
}
The thing that bothers me is, and I might be just nitpicking, but this doesn't seem like a very clean solution to me. I'm using a loop for something that is only really intended to be done once. There's an awful lot of code involved in implementing the retry stuff, and I don't think the code reads clearly. I think the presence of the loop and the control variables draw the attention of a reader / review / maintenance programmer away from the fact that what we're really trying to do is call foo(), preferably only once.
My other though for a solution is something like this:
public abstract class RetryableMethodCall {
public abstract void executeMethod();
public void attemptMethod(int maxRetrys) {
int accumulatedRetrys = 0;
while (true) {
try {
executeMethod();
break;
} catch (FooException e) {
if (++accumulatedRetrys >= maxRetrys) {
throw new BarException();
}
}
}
}
}
public class Foo {
public void someMethod() {
// ... some code that happens first ....
RetryableMethodCall rmc = new RetryableMethodCall() {
public void executeMethod() { foo(); } }
rmc.attemptMethod(3);
}
}
This solution looks nice until you have to start addressing problems like passing parameters (which need to be final to pass them to the anonymous class), and having return values, and retrying only on certain kinds of exceptions, in which case, it starts ballooning into a LOT of extra code that clutters things up pretty badly.
Does anybody have any comments or other ideas? (I'd particularly like to hear from Jos, Rene, and Jeff, if any of you are on the forum right now...)
Thanks,
Adam