Retry Runnable java

Question

I want to make class to retry doing what I want if specific exception happen for n times before throwing exception, so I write below class and used Builder to build it could you please review it and update me with your feedback:

public class Retry implements Runnable {
private final long sleep;
private final Runnable r;
private final int times;
private final Predicate<Throwable> p;
private AtomicInteger counter = new AtomicInteger();
private Retry(Builder b) {
 this.r = b.r;
 this.times = b.times;
 this.p = b.p;
 this.sleep = b.sleep;
}
@Override
public void run() {
 try {
 r.run();
 } catch (Throwable th) {
 if (counter.getAndIncrement() < times && p.test(th))
 handle(th);
 else
 throw th;
 }
}
private void handle(Throwable th) {
 System.out.println("handel #"+counter.get());
 try {
 Thread.sleep(sleep);
 } catch (InterruptedException e) {
 Thread.currentThread().interrupt();
 throw new RuntimeException(e);
 }
 run();
}
public static class Builder {
 private final static long SLEEP = 1000;
 private final static int TIMES = 5;
 private final Runnable r;
 private int times;
 private long sleep;
 private Predicate<Throwable> p;
 public Builder(Runnable r) {
 this.r = r;
 }
 public static Builder of(Runnable r) {
 return new Builder(r);
 }
 public Builder times(int times) {
 this.times = times;
 return this;
 }
 public Builder sleep(long sleep) {
 this.sleep = sleep;
 return this;
 }
 public Builder on(Predicate<Throwable> p) {
 this.p = p;
 return this;
 }
 public Retry build() {
 if (this.sleep <= 0)
 this.sleep = SLEEP;
 if (this.times <= 0)
 this.times = TIMES;
 if (this.p == null)
 this.p = th -> true;
 return new Retry(this);
 }
}
}

how to use it

public static void main(String[] args) {
Runnable r = () -> {
 throw new IllegalStateException("Test");
};
runAsync(
 Retry.Builder.of(r).times(10)
 .on(th -> th instanceof IllegalStateException).build())
 .join();
}

Answer 1

Defaulting to properties inside the Builder

You should perform validation inside the times(int), sleep(int) and on(Predicate) to either warn callers of invalid values, if not to throw an Exception. Even if you don't and prefer to silently use the default values, this is what you can do:

public Builder times(int times) {
 this.times = times <= 0 ? TIMES : times;
 return this;
}
public Builder sleep(long sleep) {
 this.sleep = sleep <= 0 ? SLEEP : sleep;
 return this;
}
public Builder on(Predicate<Throwable> p) {
 this.p = Optional.ofNullable(p).orElse(th -> true);
 return this;
}
public Retry build() {
 return new Retry(this);
}

Retry logic

This is roughly how it looks like currently:

Retry.run();
 r.run();
 |_ (Exception) handle(th);
 |_ (after sleeping) r.run();

The problem here is that you will simply have a cascading chain of calling r.run() on the method call stack, which may result in StackOverflowError if it gets too deep.

You should consider doing the retrying within run() itself as such:

@Override
public void run() {
 // edit: should be < times so that it doesn't run for the (times + 1)-th time
 // while (counter.getAndIncrement() <= times) {
 while (counter.getAndIncrement() < times) {
 try {
 r.run();
 } catch (Throwable th) {
 if (counter.get() <= times && p.test(th)) {
 handle(th);
 } else {
 throw th;
 }
 }
 }
}
private void handle(Throwable th) {
 // corrected typo below
 System.out.println("handle #" + counter.get());
 try {
 Thread.sleep(sleep);
 } catch (InterruptedException e) {
 Thread.currentThread().interrupt();
 throw new RuntimeException(e);
 }
 // don't need to call again
 // run();
}

Design

Not much to comment here, but here is one open-ended question to ponder about:

1. Is it always safe to call r.run() again for your use cases?

In other words, are there things to clean up/reinitialize across restarts?

• Bassem Reda Zohdy: good feedback I'll take it in my consideration, but there is 2 points: 1- there is no need to check '(counter.get() <= times)' again while is enough, 2- the handle method was just log and sleep I can move it inside run.
• Bassem Reda Zohdy: for clean up/reinit thinking for make beforeRunnable to be executed before each r.run(), will update my code and upload it.
• h.j.k.: @BassemRedaZohdy the second check is to make sure th is properly thrown after the last attempt. :)
• h.j.k.: @BassemRedaZohdy btw see edited code, think I got the while() condition wrong initially.