views:

140

answers:

6

I have a class that is a "manager" sort of class. One of it's functions is to signal that the long running process of the class should shut down. It does this by setting a boolean called "IsStopping" in class.

public class Foo
{
    bool isStoping

    void DoWork() {
        while (!isStopping)
        {
            // do work...
        }
    }
}

Now, DoWork() was a gigantic function, and I decided to refactor it out and as part of the process broke some of it into other classes. The problem is, Some of these classes also have long running functions that need to check if isStopping is true.

public class Foo
{
    bool isStoping

    void DoWork() {
        while (!isStopping)
        {
            MoreWork mw = new MoreWork()
            mw.DoMoreWork() // possibly long running
            // do work...
        }
    }
}

What are my options here?

I have considered passing isStopping by reference, which I don't really like because it requires there to be an outside object. I would prefer to make the additional classes as stand alone and dependancy free as possible.

I have also considered making isStopping a property, and then then having it call an event that the inner classes could be subscribed to, but this seems overly complex.

Another option was to create a "Process Cancelation Token" class, similar to what .net 4 Tasks use, then that token be passed to those classes.

How have you handled this situation?

EDIT:

Also consider that MoreWork might have a EvenMoreWork object that it instantiates and calls a potentially long running method on... and so on. I guess what i'm looking for is a way to be able to signal an arbitrary number of objects down a call tree to tell them to stop what they're doing and clean up and return.

EDIT2:

Thanks for the responses so far. Seems like there's no real consensus on methods to use, and everyone has a different opinion. Seems like this should be a design pattern...

A: 

I think firing an event that your subclasses subscribe to makes sense.

rrrhys
A: 

You could create a Cancel() method on your manager class, and on each of your other worker classes. Base it on an interface.

The manager class, or classes that instantiate other worker classes, would have to propagate the Cancel() call to the objects they are composed of.

The deepest nested classes would then just set an internal _isStopping bool to false and your long-running tasks would check for that.

Alternatively, you could maybe create a context of some sort that all the classes know about and where they can check for a canceled flag.

Another option was to create a "Process Cancelation Token" class, similar to what .net 4 Tasks use, then that token be passed to those classes.

I am not familiar with this, but if it is basically an object with a bool property flag, and that you pass into each class, then this seems like the cleanest way to me. Then you could make an abstract base class that has a constructor that takes this in and sets it to a private member variable. Then your process loops can just check that for cancellation. Obviously you will have to keep a reference to this object you have passed into your workers so that it's bool flag can be set on it from your UI.

Jacques Bosch
+5  A: 

You can go two ways here:

1) The solution you've already outlined: pass a signaling mechanism to your subordinate objects: a bool (by ref), the parent object itself cloaked in an interface (Foo: IController in the example below), or something else. The child objects check the signal as needed.

// Either in the MoreWork constructor
public MoreWork(IController controller) {
    this.controller = controller;
}

// Or in DoMoreWork, depending on your preferences
public void DoMoreWork(IController controller) {
    do {
        // More work here
    } while (!controller.IsStopping);
}

2) Turn it around and use the observer pattern - which will let you decouple your subordinate objects from the parent. If I were doing it by hand (instead of using events), I'd modify my subordinate classes to implement an IStoppable interface, and make my manager class tell them when to stop:

public interface IStoppable {
    void Stop();
}

public class MoreWork: IStoppable {
    bool isStopping = false;
    public void Stop() { isStopping = true; }
    public void DoMoreWork() {
        do {
            // More work here
        } while (!isStopping);
    }
}

Foo maintains a list of its stoppables and in its own stop method, stops them all:

public void Stop() {
    this.isStopping = true;
    foreach(IStoppable stoppable in stoppables) {
        stoppable.Stop();
    }
}
Jeff Sternal
A: 

Litter your code with statements like this wherever it is most sensible to check the stop flag:

if(isStopping) { throw new OperationCanceledException(); }

Catch OperationCanceledException right at the top level.

There is no real performance penalty for this because (a) it won't happen very often, and (b) when it does happen, it only happens once.

This method also works well in conjunction with a WinForms BackgroundWorker component. The worker will automatically catch a thrown exception in the worker thread and marshal it back to the UI thread. You just have to check the type of the e.Error property, e.g.:

private void worker_RunWorkerCompleted(object sender, RunWorkerCompletedEventArgs e) {
    if(e.Error == null) {
        // Finished
    } else if(e.Error is OperationCanceledException) {
        // Cancelled
    } else {
        // Genuine error - maybe display some UI?
    }
}
Christian Hayter
You could reuse `OperationCanceledException` instead of creating a new exception class.
CodeSavvyGeek
Thanks CodeSavvyGeek, you've saved me some coding. :-)
Christian Hayter
Updated my answer to use `OperationCanceledException`...
Christian Hayter
If you are using .net 4 there is a very good premade structure http://msdn.microsoft.com/en-us/library/dd384802.aspx just call myToken.ThrowIfCancellationRequested() and it does the above code that Christian posted. It has some other nice features like regestering events that will be fired to perform some cleanup duties.
Scott Chamberlain
See, the .net 4 CancelationToken approach bothers me. It's been drilled into me that exceptions are for exceptional conditions, not to be used as gotos. If you expect something to happen (and ending is an expected condition that will always happen) then it shouldn't be use an exception. I can "kind of" see canceling a process before it's finished normally as exceptional, but for finishing the process... I'm just not too thrilled with this approach. Yes, I know I said "canceling" in my post, i should have more carefully delineated the two. I need to do both.
Mystere Man
A: 

Your nested types could accept a delegate (or expose an event) to check for a cancel condition. Your manager then supplies a delegate to the nested types that checks its own "shouldStop" boolean. This way, the only dependency is of the ManagerType on the NestedType, which you already had anyway.

class NestedType
{
    // note: the argument of Predicate<T> is not used, 
    //    you could create a new delegate type that accepts no arguments 
    //    and returns T
    public Predicate<bool> ShouldStop = delegate() { return false; };
    public void DoWork()
    {
        while (!this.ShouldStop(false))
        {
            // do work here
        }
    }
}

class ManagerType
{
    private bool shouldStop = false;
    private bool checkShouldStop(bool ignored)
    {
        return shouldStop;
    }
    public void ManageStuff()
    {
        NestedType nestedType = new NestedType();
        nestedType.ShouldStop = checkShouldStop;
        nestedType.DoWork();
    }
}

You could abstract this behavior into an interface if you really wanted to.

interface IStoppable
{
    Predicate<bool> ShouldStop;
}

Also, rather than just check a boolean, you could have the "stop" mechanism be throwing an exception. In the manager's checkShouldStop method, it could simply throw an OperationCanceledException:

class NestedType
{
    public MethodInvoker Stop = delegate() { };
    public void DoWork()
    {
        while (true)
        {
            Stop();
            // do work here
        }
    }
}

class ManagerType
{
    private bool shouldStop = false;
    private void checkShouldStop()
    {
        if (this.shouldStop) { throw new OperationCanceledException(); }
    }
    public void ManageStuff()
    {
        NestedType nestedType = new NestedType();
        nestedType.Stop = checkShouldStop;
        nestedType.DoWork();
    }
}

I've used this technique before and find it very effective.

CodeSavvyGeek
A: 

You can flatten your call stack by turning each DoWork() call into a command using the Command pattern. At the top level, you maintain a queue of commands to perform (or a stack, depending on how your commands interact with each other). "Calling" a function is translated to enqueuing a new command onto the queue. Then, between processing each command, you can check whether or not to cancel. Like:

void DoWork() {
    var commands = new Queue<ICommand>();

    commands.Enqueue(new MoreWorkCommand());
    while (!isStopping && !commands.IsEmpty)
    {
        commands.Deque().Perform(commands);
    }
}

public class MoreWorkCommand : ICommand {
    public void Perform(Queue<ICommand> commands) {
        commands.Enqueue(new DoMoreWorkCommand());
    }
}

Basically, by turning the low-level callstack into a data structure you control, you have the ability to check stuff between each "call", pause, resume, cancel, etc..

munificent