Question

Working on a project where a sequential set of methods must be run every x seconds. Right now I have the methods contained within another "parent method", and just sequentially call them right after another.

class DoTheseThings()
{
    DoThis();
    NowDoThat();
    NowDoThis();
    MoreWork();
    AndImSpent();
}

Each method must run successfully without throwing an exception before the next step can be done. So now I wrapped each of those methods with a while and try..catch, then in the catch execute that method again.

while( !hadError )
{
    try
    {
         DoThis();
    }
    catch(Exception doThisException )
    {
         hadError = true;
    }

}

This seems smelly and not very dry. Is there a better way to do this so I'm not wrapping any new functionality in the same methods. Isn't some kind of Delegate collection the proper way to implement this?

Is there a more "proper" solution?

Was it helpful?

Solution

Action[] work=new Action[]{new Action(DoThis),   new Action(NowDoThat),    
    new Action(NowDoThis),    new Action(MoreWork),    new Action(AndImSpent)};
int current =0;
while(current!=work.Length)
{
   try
   {
      work[current]();
      current++;
   }
   catch(Exception ex)
   {
      // log the error or whatever
      // maybe sleep a while to not kill the processors if a successful execution depends on time elapsed  
   }
}

OTHER TIPS

Isn't some kind of Delegate collection the proper way to implement this?

Delegate is a possible way to solve this problem.

Just create a delegate something like:

public delegate void WorkDelegate();

and put them in arraylist which you can iterate over.

I have a personal religious belief that you shouldn't catch System.Exception, or more accurately, you should only catch the exceptions you know how to handle.

That being said, I am going to assume that each one of the methods that you are calling are doing something different, and could result in different exceptions being thrown. Which means you would likely need to have different handlers for each method.

If you follow my religion as well, and the second statement is true, then you are not repeating code unnecessarily. Unless you have other requirements, my recommendations to improve your code would be:

1) Put the try-catch in each method, not around each method call.

2) Have the catches within each method catch ONLY the exceptions you know about.

http://blogs.msdn.com/fxcop/archive/2006/06/14/631923.aspx http://blogs.msdn.com/oldnewthing/archive/2005/01/14/352949.aspx http://www.joelonsoftware.com/articles/Wrong.html

HTH ...

your example seems ok.. its a dry one but will do the job well!! actually if this methods execute db access.. you can use transaction to ensure integrity...

if your dealing with shared variables for multi threader programs.. it is cleaner to use synchronization.. the most important thing in coding is that you write the proper code... that has less bugs.. and will do the task correctly..

public void DoTheseThings()
{
    SafelyDoEach( new Action[]{
        DoThis,
        NowDoThat,
        NowDoThis,
        MoreWork,
        AndImSpent
    })
}

public void SafelyDoEach( params Action[] actions )
{
    try
    {
        foreach( var a in actions )
            a();
    }
    catch( Exception doThisException )
    {
        // blindly swallowing every exception like this is a terrible idea
        // you should really only be swallowing a specific MyAbortedException type
        return;
    }
}

What would be the reason that an error was occuring?

If this were a resource issue, such as access to something like a connection or object, then you might want to look at using monitors, semaphores, or just locking.

lock (resource) 
{
    Dosomething(resource);
}

This way if a previous method is accessing the resource, then you can wait until it releases the resource to continue.

Ideally, you shouldn't have to run a loop to execute something each time it fails. It is failing at all, you would want to know about the issue and fix it. Having a loop to always just keep trying is not the right way to go here.

I'd do what Ovidiu Pacurar suggests, only I'd use a foreach loop and leave dealing with array indexes up to the compiler.

Simple delegate approach:

Action<Action> tryForever = (action) => { 
   bool success;
   do {
     try {
       action();
       success = true;
     } catch (Exception) {
       // should probably log or something here...
     }
   } while (!success);
};

void DoEverything() {
   tryForever(DoThis);
   tryForever(NowDoThat);
   tryForever(NowDoThis);
   tryForever(MoreWork);
   tryForever(AndImSpent);
}

Stack approach:

void DoEverything() {
  Stack<Action> thingsToDo = new Stack<Action>(
    new Action[] { 
      DoThis, NowDoThat, NowDoThis, MoreWork, AndImSpent 
    }
  );

  Action action;
  while ((action = thingsToDo.Pop()) != null) {
     bool success;
     do {
       try {
         action();
         success = true;
       } catch (Exception) {
       }
     } while (!success);
  }
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top