1

Note: This is toy code and not production ready.

I want to schedule MyTask to run every fixed delay (say 2 seconds for example). And this task when done wants itself to be stopped. The code for MyTask is:

public class MyTask implements Runnable {
    MainClass parent;
    AtomicInteger integer = new AtomicInteger(0);
    public MyTask(MainClass parent) {
        this.parent = parent;
    }

    @Override
    public void run() {
        try {
            int valueNow = integer.incrementAndGet();
            System.out.println("Running with value: " + valueNow + " and going to do work");
            Thread.sleep((long)(Math.random() * 10000)); // simulate some work
            System.out.println("Running with value: " + valueNow + " and work over");
            if(valueNow == 5) {
                parent.stopTask();
            }
        } catch (Exception exception) {
            System.out.println("Interrupted");
        }
    }
}

And the MainClass is the one which schedules it:

public class MainClass {
    private ScheduledExecutorService executorService;
    private ScheduledFuture updateFuture;
    public static void main(String[] args) {
        new MainClass().startMyTask();
    }
    public void startMyTask() {
        System.out.println("Starting MyTask to run every 2 seconds..............");
        executorService = Executors.newScheduledThreadPool(2);
        updateFuture = executorService.scheduleAtFixedRate(new MyTask(this), 1, 2, TimeUnit.SECONDS);
    }
    public void stopTask() {
       System.out.println("Stopping MyTask to run further");
       updateFuture.cancel(true);
    }
}

This is just a toy code to reproduce the scenario. Is there anything wrong with this approach where I am passing a reference to the parent to the thread so that it can be stopped? Is there any better approach for this?

akhil_mittal
  • 23,309
  • 7
  • 96
  • 95

1 Answers1

0

Well, what is 'wrong' is that you create a cyclic dependency between the two classes. It'll work but cyclic dependencies are generally undesirable.

But it's easy enough to break the cycle. Rather than passing parent itself, your task can accept a Runnable to run when it is done; basically a callback.

public class MyTask implements Runnable {
    Runnable runOnDone;
    AtomicInteger integer = new AtomicInteger(0);
    public MyTask(Runnable runOnDone) {
        this.runOnDone = runOnDone;
    }

    @Override
    public void run() {
        try {
            int valueNow = integer.incrementAndGet();
            System.out.println("Running with value: " + valueNow + " and going to do work");
            Thread.sleep((long)(Math.random() * 10000)); // simulate some work
            System.out.println("Running with value: " + valueNow + " and work over");
            if(valueNow == 5) {
                runOnDone.run();
            }
        } catch (Exception exception) {
            System.out.println("Interrupted");
        }
    }
}

The task can then be instantiated like this : new MyTask(this::stopTask)

Alternatively you can throw an exception from your task, which will also end it being rescheduled. See this stackoverflow answer.

bowmore
  • 10,842
  • 1
  • 35
  • 43
  • The condition `valueNow==5` is to stop the task from running further in next cycle. Why do you want to invoke one more runnable? I want to stop the current one from further running. – akhil_mittal Nov 29 '17 at 08:41
  • I'm not spawning a new thread. I'm running the runnable on the same thread. – bowmore Nov 29 '17 at 08:42
  • The `Runnable` is just an abstraction for a bit of code to run, when done. – bowmore Nov 29 '17 at 08:42
  • Yes I mean in the same thread and not a new one, my bad. – akhil_mittal Nov 29 '17 at 08:44
  • And by passing `this::stopTask` to MyTask, it does exactly the same as it does in your version, except that it doesn't have an explicit dependency on `MainClass`, i.e. you can launch `MyTask` from other classes than `MainClass` – bowmore Nov 29 '17 at 08:47
  • Yes it will work but I just want to see if there is any better approach to achieve this. – akhil_mittal Nov 29 '17 at 08:48
  • Its better than mine :) but I will wait if someone comes up with even better approach. – akhil_mittal Nov 29 '17 at 08:49
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/160064/discussion-between-i-am-zero-and-bowmore). – akhil_mittal Nov 29 '17 at 08:51