Simple but important function to check and retry given action's result

Question

What I'm trying to achieve is Given a predicate function and an action which returns the object to check, check predicate, retry and fail after numTries .. or success .

Purpose of this function is :

During constant data collection from network resources ( web, apis, .. ) return of network operations not successful every time ( because of bad servers, network problems , etc.. ). If you need that data you should retry, and if you do this kind of ops. a lot, you should have a function to handle this.

I'm looking for are there better ways to do this.

public static async Task<bool> CheckAndRetryAsync<T>
        (
            T _object, //object to check state
            Func<T, bool> predicate, Func<Task<T>> action, // object creation func
            int sleep = 150, // sleep ms. between tries
            int numTries = 6, // ..
            bool rethrowExceptions = false, // should rethrow exc. during object creation
            ManualResetEvent mr = null
        )
    {
        int tries = 0;
        bool success = false;
        while (true)
        {
            try
            {
                _object = await action();
                success = predicate(_object);
                if (!success && tries < numTries)
                {
                    tries++;
                    await Task.Delay(sleep);
                }
                else
                {
                    //goto exit; 
                    break;
                }
            }
            catch (Exception e)
            {
                if (rethrowExceptions)
                    throw;
                if (tries < numTries){
                    tries++;
                    await Task.Delay(sleep);
                }
                else{
                    goto exit;
                }

            }
        }
        exit:
        if(mr != null)
            mr.Set();
        return success;
    }

Tests for function :

public class StaticUtilFuncTests
{
    ManualResetEvent mr = new ManualResetEvent(false);
    [Fact]
    public async Task T_010_CheckAndRetryAsync()
    {
        int tries = 1;
        mr.Reset();
        Stopwatch sw = new Stopwatch();
        sw.Start();
        TObj ttx = null;
        bool succsess = await CheckAndRetryAsync<TObj>(ttx,
            (o) => (o != null && o.Value != ""),
            async () =>
            {
                if (tries <= 4)
                {
                    ttx = new TObj();
                    tries++;
                }
                else
                {
                    ttx = new TObj("sdf");

                }
                mr.Set();

                return ttx;

            },
            numTries: 5, sleep: 400
            );
        mr.WaitOne();
        long elapsed = sw.ElapsedMilliseconds;
        Assert.True(elapsed >= 1600l); // numtries * sleep
        Assert.True(succsess);
        Assert.True(tries == 5);

    }
    [Fact]
    public async Task T_011_CheckAndRetryAsync()
    {
        mr.Reset();

        Stopwatch sw = new Stopwatch();
        sw.Start();
        TObj ttx = null;
        bool succsess = await CheckAndRetryAsync<TObj>(
            ttx,
            (o) => (o != null && o.Value != ""),
            async () =>
            {

                ttx = new TObj("");
                mr.Set();

                return ttx;
            },
            numTries: 3, sleep: 1100
            );
        mr.WaitOne();
        long elapsed = sw.ElapsedMilliseconds;
        Assert.True(elapsed >= 3300l); // numtries * sleep
        Assert.False(succsess);
    }
[Fact]
    public async Task T_012_CheckAndRetryAsync()
    {
        mr.Reset();
        //Mock<TObj> to = new Mock<TObj>();
        //to.Setup(
        //    m => m.)

        Stopwatch sw = new Stopwatch();
        sw.Start();
        TObj ttx = null;
        bool succsess = await CheckAndRetryAsync<TObj>(
            ttx,
            (o) => (o != null && o.Value != ""),
            async () =>
            {

                ttx = new TObj("",true);
                //mr.Set();

                return ttx;
            },
            numTries: 30, sleep: 50, mr: mr
            );
        mr.WaitOne();
        long elapsed = sw.ElapsedMilliseconds;
        Assert.True(elapsed >= 1500l); // numtries * sleep
        Assert.False(succsess);
    }
}
public class TObj
{
    public string Value;

    public TObj(string value = "" , bool testing = false)
    {
        if(value == "" && testing)
            throw new Exception();
        Value = value;
    }
}
}

A real usage example :

 BittrexBtcTicker btcTicker = null;
        success |= await CheckAndRetryAsync<BittrexBtcTicker>(
            btcTicker, // object we need to get ( also save db )
            (o) => (o != null && o.Value > 0), // predicate not null, and bigger than 0
            async () =>
            {
                mr.Reset(); // a noise but needed during async ops.
                btcTicker = await BittrexClient
                    .GetTicker(saveDb: true, updateDb: updateDb, _context: context, mr: mr); // call to object creation..
                mr.WaitOne(); // wait to op. complete 
                return btcTicker;
            }
            );

Answer 1

Design

• I see you fixed the bug where the method would get stuck in an infinite loop when action throws an exception, but note how similar the normal and the exception paths are now. The method can be simplified by incrementing tries before invoking the action and by awaiting the delay after (outside) the try-catch. Try to keep code DRY (Don't Repeat Yourself).
• More simplifications include using a proper while condition: while (tries < numTries), to make the intent of the loop more clear, and the success check can be simplified to if (predicate(result)) break;.
• There's no need for goto here - break is sufficient (and easier to understand).
• The T _object parameter is useless. Currently, the only way to obtain a result is to use a closure. I assume you meant to make this a ref parameter? If so, I would rename it to TResult result. Alternately, you could return a (bool success, TResult result) value-tuple, which can be used like var (success, result) = await CheckAndRetryAsync(...);
• You may want to add ConfigureAwait(false) to your awaits, unless resuming in the same context is important.
• Are you sure you want to ignore exceptions by default? Including the last try? No logging even?
• What's the point of passing a ManualResetEvent into this method? Why not just do that in awaiting code: await CheckAndRetryAsync(...); mr.Set();?
• You may want to enforce that sleep is not negative.

Tests

• The test method names do not describe what they're testing, and none of the Assert statements contain a descriptive error message. That's not good for future maintenance.
• There's a lot of code duplication in the tests. You may want to refactor it into a data-driven approach, or write a utility method for the repetitive parts.
• What's the purpose of that ManualResetEvent? Why is it shared between all tests? You may want to add a comment explaining why it's there.
• Throwing an exception from a constructor (TObj) instead of throwing it directly inside a test method is making the tests harder to understand.
• You may want to add a timeout to your tests, to ensure that infinite loops and other such problems will get caught.
• Instead of comparing elapsed against a magic number, I'd create local variables for the number of tries and timeout, so the minimum elapsed time can be calculated from them.
• In T_010, tries will always end up being 5, even if action gets called more than 5 times.
• There's no test that checks whether exceptions are rethrown.

Style/readability

• All parameters are on a line of their own, except the two most important ones, so at first sight it looks like there's only one Func parameter. That's confusing!
• Personally I'd put action before predicate, to match execution order (you first need a result before you can check it, after all).
• Add some whitespace between methods and between blocks of code inside methods to improve readability.
• The l prefix looks a lot like a 1 - consider using L instead, or just leave it out.