current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 1 down vote favorite

I have to store and return data on multitasking requests. If data is missing, I schedule loading and return null. I'm using some nubie scheduling by saving current jobs in a list. Also I need data request to be locked during writing. .NET 4.0

How can I schedule and lock properly?

private ConcurrentDictionary<string, IDataSet> data;
private List<string> loadingNow;

public IDataSet GetData(string dataId)
{
    if (loadingNow.Contains(dataId))
        return null; // Currently loading. Return null

    if (data.ContainsKey(dataId))
        return data[dataId]; // Return data

    // Schedule loading async. Return null.
    loadingNow.Add(dataId);
    dataIoAsync.LoadDataAsync(dataParams);
    return null;
}

private void DataIoAsync_DataFileLoaded(object sender, DataFileLoadedAsyncEventArgs e)
{
    loadingNow.Remove(e.DataId);
    data.TryAdd(e.DataId, e.DataSet);
    OnDataFileLoaded(e.DataId);
}

EDIT: Dictionary replaced with ConcurrentDictionary

share|improve this question

1 Answer 1

up vote 1 down vote accepted

Instead of keeping track of what's being loaded, you can represent that as a state by wrapping IDataSet into a class:

private sealed class DataSetResult
{
    public volatile IDataSet Result;
}

When an instance of this class is created, start loading the data. You can then just return the value of the Result field - if it is null, the data is still being loaded:

private readonly ConcurrentDictionary<string, Lazy<DataSetResult>> _data;

public IDataSet GetData(string dataId)
{
    var dataSet = _data.GetOrAdd(
        dataId, 
        _ => new Lazy<DataSetResult>(
            () =>
            {
                var result = new DataSetResult();
                dataIoAsync.LoadDataAsync(dataParams);
                return result;
            }
        )
    );

    return dataSet.Value.Result;
}

private void DataIoAsync_DataFileLoaded(object sender, DataFileLoadedAsyncEventArgs e)
{
    _data[e.DataId].Value.Result = e.DataSet;
    OnDataFileLoaded(e.DataId);
}

The Lazy<DataSetResult> above ensures that data is retrieved at most once for a given id.

Personally, however, I would prefer to return Task<IDataSet> from this method, as it is really up to the caller to decide whether they want to wait for the result to become available or not. Using Task also allows you to report any exceptions to the client:

private readonly ConcurrentDictionary<string, Lazy<TaskCompletionSource<IDataSet>>> _data;

public Task<IDataSet> GetDataAsync(string dataId)
{
    var dataSet = _data.GetOrAdd(
        dataId, 
        _ => new Lazy<TaskCompletionSource<IDataSet>>(
            () =>
            {
                var result = new TaskCompletionSource<IDataSet>();
                dataIoAsync.LoadDataAsync(dataParams);
                return result;
            }
        )
    );

    return dataSet.Value.Task;
}

private void DataIoAsync_DataFileLoaded(object sender, DataFileLoadedAsyncEventArgs e)
{
    _data[e.DataId].Value.TrySetResult(e.DataSet);
    OnDataFileLoaded(e.DataId);
}
share|improve this answer
    
It's very interesting. I have tried something similar. Storing null dataSet in _data, but there was a side effect. This wrapping may solve the problem. I have to read further about Lazy<DataSetResult>. I'll test your solution. –  Miroslav Popov Jul 7 '13 at 7:15

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.