2

I have 2 data sources: online and offline (cached). Both of them returns IObservable of object which contains 2 flags - IsSuccess and IsCached. I would like to get data from online source but only when IsSuccess=true. If this fail I would like to get data from offline source. Additionally I want to save new data in cache for future. I am not sure how to do it best in RX. Here is my implementation of that but I think it can be done much better

public IObservable<Result<SampleModel>> GetSampleModel()
    {
        IObservable<Result<SampleModel>> onlineObservable = _onlineSource.GetData<SampleModel>();
        IObservable<Result<SampleModel>> offlineObservable = _offlineSource.GetData<SampleModel>();

        var subject = new Subject<Result<SampleModel>>();

        onlineObservable.Do(async (result) =>
        {
            if (result.IsSuccess)
            {
                await _offlineSource.CacheData(result.Data).ConfigureAwait(false);
            }
        }).Subscribe((result) =>
        {
            if (result.IsSuccess)
            {
                subject.OnNext(result);
            }
            subject.OnCompleted();
        });

        return subject.Concat(offlineObservable).Take(1);
    }

Result class - wrapper for data:

public class Result<T>
{
    public Result(Exception exception)
    {
        Exception = exception;
    }

    public Result(T data, bool isCached = false)
    {
        IsCached = isCached;
        IsSuccess = true;
        Data = data;
    }

    public bool IsSuccess { get; private set; }
    public bool IsCached { get; private set; }
    public T Data { get; private set; }
    public Exception Exception { get; private set; }
}
Tomasz Pikć
  • 753
  • 5
  • 21

1 Answers1

1

Your implementation will not work reliably, because there is a race condition in there. Consider this:

var temp = GetSampleModel(); // #1
// Do some long operation here
temp.Subscribe(p => Console.WriteLine(p)); // #2

In this case, fetching data will start in #1, and if the data is received and pushed to subject before #2 executes, nothing will be printed no matter how long you wait.

Usually, you should avoid subscribing inside a function returning IObservable to avoid such issues. Using Do is also a bad smell. You could fix the code using ReplaySubject or AsyncSubject, but in such cases I generally prefer Observable.Create. Here is my rewrite:

public IObservable<SampleModel> GetSampleModel(IScheduler scheduler = null)
{
    scheduler = scheduler ?? TaskPoolScheduler.Default;
    return Observable.Create<SampleModel>(observer =>
    {
        return scheduler.ScheduleAsync(async (s, ct) =>
        {
            var onlineResult = await _onlineSource.GetData<SampleModel>().FirstAsync();
            if (onlineResult.IsSuccess)
            {
                observer.OnNext(onlineResult.Data);
                await _offlineSource.CacheData(onlineResult.Data);
                observer.OnCompleted();
            }
            else
            {
                var offlineResult = await _offlineSource.GetData<SampleModel>().FirstAsync();
                if (offlineResult.IsSuccess)
                {
                    observer.OnNext(offlineResult.Data);
                    observer.OnCompleted();
                }
                else
                {
                    observer.OnError(new Exception("Could not receive model"));
                }
            }
            return Disposable.Empty;
        });
    });
}

You can see that it still isn't terribly pretty. I think that it's because you chose not to use natural Rx system of handling errors, but instead to wrap your values in Result type. If you alter your repository methods to handle errors in Rx way, resulting code is much more concise. (Note that I changed your Result type to MaybeCached, and I assume that now both sources return IObservable<SampleModel>, which is a cold observable either returning a single result or an error):

public class MaybeCached<T>
{
    public MaybeCached(T data, bool isCached)
    {
        IsCached = isCached;
        IsSuccess = true;
    }

    public bool IsCached { get; private set; }
    public T Data { get; private set; }
}

public IObservable<SampleModel> GetSampleModel()
{
    _onlineSource
        .GetData<SampleModel>()
        .Select(d => new MaybeCached(d, false))
        .Catch(_offlineSource
                    .GetData<SampleModel>()
                    .Select(d => new MaybeCached(d, true))
        .SelectMany(data => data.IsCached ? Observable.Return(data.Data) : _offlineSource.CacheData(data.Data).Select(_ => data.Data));
}

Catch is used here in order to obtain a conditional switch you asked for.

pmbanka
  • 1,902
  • 17
  • 24