I have a Runnable. I have a class that schedules this Runnable for execution using a ScheduledExecutorService with scheduleWithFixedDelay.
I want to alter this class to schedule the Runnable for fixed delay execution either indefinitely, or until it has been run a certain number of times, depending on some parameter that is passed in to the constructor.
If possible, I would like to use the same Runnable, as it is conceptually the same thing that should be "run".
Have two Runnables, one that cancels the schedule after a number of executions (which it keeps a count of) and one that doesn't:
public class MyClass{
private ScheduledExecutorService scheduler = Executors.newSingleThreadScheduledExecutor();
public enum Mode{
INDEFINITE, FIXED_NO_OF_TIMES
}
public MyClass(Mode mode){
if(mode == Mode.INDEFINITE){
scheduler.scheduleWithFixedDelay(new DoSomethingTask(), 0, 100, TimeUnit.MILLISECONDS);
}else if(mode == Mode.FIXED_NO_OF_TIMES){
scheduler.scheduleWithFixedDelay(new DoSomethingNTimesTask(), 0, 100, TimeUnit.MILLISECONDS);
}
}
private class DoSomethingTask implements Runnable{
@Override
public void run(){
doSomething();
}
}
private class DoSomethingNTimesTask implements Runnable{
private int count = 0;
@Override
public void run(){
doSomething();
count++;
if(count > 42){
// Cancel the scheduling.
// Can you do this inside the run method, presumably using
// the Future returned by the schedule method? Is it a good idea?
}
}
}
private void doSomething(){
// do something
}
}
I would rather just have one Runnable for the execution of the doSomething method. Tying the scheduling to the Runnable feels wrong. What do you think about this?
Have a single Runnable for the execution of the code that we want to run periodically. Have a separate scheduled runnable that checks how many times the first Runnable has run and cancels when it gets to a certain amount. This may not be accurate, as it would be asynchronous. It feels a bit cumbersome. What do you think about this?
Extend ScheduledExecutorService and add a method "scheduleWithFixedDelayNTimes". Perhaps such a class already exists? Currently, I'm using Executors.newSingleThreadScheduledExecutor();
to get my ScheduledExecutorService instance. I would presumably have to implement similar functionality to instantiate the extended ScheduledExecutorService. This could be tricky. What do you think about this?
I could not use a scheduler. I could instead have something like:
for(int i = 0; i < numTimesToRun; i++){
doSomething();
Thread.sleep(delay);
}
And run that in some thread. What do you think of that? You could potentially still use the runnable and call the run method directly.
Any suggestions welcome. I'm looking for a debate to find the "best practice" way of achieving my goal.
You can use the cancel() method on Future. From the javadocs of scheduleAtFixedRate
Otherwise, the task will only terminate via cancellation or termination of the executor
Here is some example code that wraps a Runnable in another that tracks the number of times the original was run, and cancels after running N times.
public void runNTimes(Runnable task, int maxRunCount, long period, TimeUnit unit, ScheduledExecutorService executor) {
new FixedExecutionRunnable(task, maxRunCount).runNTimes(executor, period, unit);
}
class FixedExecutionRunnable implements Runnable {
private final AtomicInteger runCount = new AtomicInteger();
private final Runnable delegate;
private volatile ScheduledFuture<?> self;
private final int maxRunCount;
public FixedExecutionRunnable(Runnable delegate, int maxRunCount) {
this.delegate = delegate;
this.maxRunCount = maxRunCount;
}
@Override
public void run() {
delegate.run();
if(runCount.incrementAndGet() == maxRunCount) {
boolean interrupted = false;
try {
while(self == null) {
try {
Thread.sleep(1);
} catch (InterruptedException e) {
interrupted = true;
}
}
self.cancel(false);
} finally {
if(interrupted) {
Thread.currentThread().interrupt();
}
}
}
}
public void runNTimes(ScheduledExecutorService executor, long period, TimeUnit unit) {
self = executor.scheduleAtFixedRate(this, 0, period, unit);
}
}