HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

A Switch for async function invocations

Submitted by: @import:stackexchange-codereview··
0
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) {

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:

public int GetFoo()
{
    // ...
}


According to the official naming guidelines all methods - public or otherwise - should be PascalCase

Why 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.