patterncsharpMinor
A Switch for async function invocations
Viewed 0 times
asyncfunctionforswitchinvocations
Problem
I have implemented a simple "switch" function - that switches and invokes an alternate function, if the function that is to be invoked fails or timesout.
This code will be later employed as part of circuit breaker.
I request to review the code in general and in particular for correct use of async\await:
```
public class Switch
{
private CancellationTokenSource combinedCancelTokenSource;
private CancellationToken cancelToken;
private CancellationToken timeCancelToken;
private Func> alternateFn;
private Action exceptionIntercept;
public Switch() {
this.combinedCancelTokenSource = new CancellationTokenSource();
}
public async Task InvokeAysnc(Func> f) {
bool computeAlternate = false;
R r = default(R);
Stopwatch watch = new Stopwatch();
try {
watch.Start();
r = await f(combinedCancelTokenSource.Token);
watch.Stop();
} catch(OperationCanceledException) {
watch.Stop();
if (timeCancelToken.IsCancellationRequested) {
// log as Fault
logTimedOut(watch.Elapsed);
computeAlternate = true;
}
//else - cancellation was requested by the callee, return default value, do not log as fault
} catch(Exception exp) {
watch.Stop();
exceptionIntercept?.Invoke(exp);
logFault(watch.Elapsed, exp);
computeAlternate = true;
}
if(computeAlternate && alternateFn != null) {
r = await alternateFn(cancelToken);
}
return r;
}
private void logFault(TimeSpan elapsed, Exception exp) {
// will implement later
}
private void logTimedOut(TimeSpan elapsed) {
This code will be later employed as part of circuit breaker.
I request to review the code in general and in particular for correct use of async\await:
```
public class Switch
{
private CancellationTokenSource combinedCancelTokenSource;
private CancellationToken cancelToken;
private CancellationToken timeCancelToken;
private Func> alternateFn;
private Action exceptionIntercept;
public Switch() {
this.combinedCancelTokenSource = new CancellationTokenSource();
}
public async Task InvokeAysnc(Func> f) {
bool computeAlternate = false;
R r = default(R);
Stopwatch watch = new Stopwatch();
try {
watch.Start();
r = await f(combinedCancelTokenSource.Token);
watch.Stop();
} catch(OperationCanceledException) {
watch.Stop();
if (timeCancelToken.IsCancellationRequested) {
// log as Fault
logTimedOut(watch.Elapsed);
computeAlternate = true;
}
//else - cancellation was requested by the callee, return default value, do not log as fault
} catch(Exception exp) {
watch.Stop();
exceptionIntercept?.Invoke(exp);
logFault(watch.Elapsed, exp);
computeAlternate = true;
}
if(computeAlternate && alternateFn != null) {
r = await alternateFn(cancelToken);
}
return r;
}
private void logFault(TimeSpan elapsed, Exception exp) {
// will implement later
}
private void logTimedOut(TimeSpan elapsed) {
Solution
I don't want to start a holy war about it but you should know that your bracing style isn't generally the expected style for C#. It is more common for the opening brace to appear on a newline:
According to the official naming guidelines all methods -
Why have you chosen
With that small housekeeping aside, here's some example code to highlight an issue with your code:
If I've read your code correctly, this will never return because the time based cancellation token will have already requested cancellation and you don't check the
It would be better to store the TimeSpan in a field and set up the time based cancellation token as the first step of your
Have you thought about how you'd reuse an instance of your switch?
I'd also rename your methods so it looks more like:
public int GetFoo()
{
// ...
}According to the official naming guidelines all methods -
public or otherwise - should be PascalCaseWhy have you chosen
R for the Type parameter? It's a small thing, but most people would expect to see T. Try to stick with convention where you can.With that small housekeeping aside, here's some example code to highlight an issue with your code:
// set up switch
var someSwitch = new Switch()
.WithinTime(TimeSpan.FromSeconds(1))
.Alternate(token => Task.FromResult(100));
// do some long work
Thread.Sleep(4000);
// use the switch
var result = await someSwitch.InvokeAsync(token =>
{
Thread.Sleep(Timeout.Infinite);
return Task.FromResult(1234);
});If I've read your code correctly, this will never return because the time based cancellation token will have already requested cancellation and you don't check the
IsCancellationRequested property at all.It would be better to store the TimeSpan in a field and set up the time based cancellation token as the first step of your
InvokeAsync method.Have you thought about how you'd reuse an instance of your switch?
I'd also rename your methods so it looks more like:
var result = await new Switch()
.WithTimeLimit(TimeSpan.FromSeconds(1))
.WithFallbackFunction(token => Task.FromResult("Fell off a cliff"))
.RunFunctionAsync(token => Task.FromResult("Hello world"));InvokeAysnc should be InvokeAsync.Code Snippets
public int GetFoo()
{
// ...
}// set up switch
var someSwitch = new Switch<int>()
.WithinTime(TimeSpan.FromSeconds(1))
.Alternate(token => Task.FromResult(100));
// do some long work
Thread.Sleep(4000);
// use the switch
var result = await someSwitch.InvokeAsync(token =>
{
Thread.Sleep(Timeout.Infinite);
return Task.FromResult(1234);
});var result = await new Switch<string>()
.WithTimeLimit(TimeSpan.FromSeconds(1))
.WithFallbackFunction(token => Task.FromResult("Fell off a cliff"))
.RunFunctionAsync(token => Task.FromResult("Hello world"));Context
StackExchange Code Review Q#135853, answer score: 2
Revisions (0)
No revisions yet.