patterncsharpMinor
Retry logic for delegates
Viewed 0 times
logicdelegatesforretry
Problem
I created this class which supposed to take a method and try to execute it. If it fails lets say 10 times, it will throw the original exception thrown by the delegate.
Do you have any ideas for improvement about that code?
I am particularly interested, in where to put this code. Right now it is within a
But since it can be used generally I want to move it to a different assembly. Unfortunately I did not come up with a better name (other than "Utility" or "Helper"). Unfortunately I did not come with a really meaningful name...maybe something related to "invoke" or so?
Do you have any ideas for improvement about that code?
I am particularly interested, in where to put this code. Right now it is within a
FileHandler assembly because the code was used for serialization.But since it can be used generally I want to move it to a different assembly. Unfortunately I did not come up with a better name (other than "Utility" or "Helper"). Unfortunately I did not come with a really meaningful name...maybe something related to "invoke" or so?
public static class FileHandler
{
private const int MAX_TRIES = 10;
private const int WAIT_FOR_RETRY_DELAY = 250;
public static T ExecuteFuncWithRetry(Func func, string errorMessage)
{
var retryFunc = new FuncWithRetry(errorMessage, func);
var ret = retryFunc.PerformAction();
return ret;
}
private class FuncWithRetry
{
private readonly string _errorMessage;
private readonly Func _func;
public FuncWithRetry(string errorMessage, Func func)
{
_errorMessage = errorMessage;
_func = func;
}
public T PerformAction()
{
var tries = 1;
Exception exception = null;
do
{
try
{
var retval = _func();
return retval;
}
catch (Exception e)
{
exception = e;
Thread.Sleep(WAIT_FOR_RETRY_DELAY);
}
finally
{
tries++;
}
} while (tries <= MAX_TRIES);
throw new Exception(_errorMessage, exception);
}
}
}Solution
Assemblies and naming
Assemblies are units of deployment -- you should put this code in the assembly it's being used in; don't over-think it. Use namespaces, not assemblies, for logical partitioning of code, and don't create a new assembly just because some code may be used elsewhere. Wait until it is used elsewhere.
However
However, I would question the usefulness of such a function.
You catch an
You throw an
What if it's being called on the UI thread, and it wouldn't make sense to call
The problem with this method is that it tries to be too general. Retry logic will often have very specific requirements that can't be abstracted away into one function. For instance, say I'm getting exceptions that contain HTTP status codes. If I'm getting 503 Service Unavailable, I may want to try again with exponential back-off. But if it's a 400 Bad Request, there is something wrong with my code and I should log it without retrying.
Assemblies are units of deployment -- you should put this code in the assembly it's being used in; don't over-think it. Use namespaces, not assemblies, for logical partitioning of code, and don't create a new assembly just because some code may be used elsewhere. Wait until it is used elsewhere.
However
However, I would question the usefulness of such a function.
You catch an
Exception, which is generally not recommended -- what happens if it's an OutOfMemoryException? But you're forced to catch an Exception as you have no way of knowing which exceptions could be thrown.You throw an
Exception, forcing callers in turn to catch an Exception. And only the last exception is kept around (in the inner exception) -- what if some exceptions require special handling, or logging? All the information contained in them is lost. As @CodesInChaos commented, an AggregateException would be better.What if it's being called on the UI thread, and it wouldn't make sense to call
Thread.Sleep between retries?The problem with this method is that it tries to be too general. Retry logic will often have very specific requirements that can't be abstracted away into one function. For instance, say I'm getting exceptions that contain HTTP status codes. If I'm getting 503 Service Unavailable, I may want to try again with exponential back-off. But if it's a 400 Bad Request, there is something wrong with my code and I should log it without retrying.
Context
StackExchange Code Review Q#53980, answer score: 4
Revisions (0)
No revisions yet.