CODEWITHSUNDEEP
c#asynchronousasync-awaittask-parallel-library
Felice Pollano
Felice Pollano

Reputation: 33272

Not sure is the correct way in handling file asynchronously

I'm using this code to write a group of files on disk:

var savingTasks = games.Games.Select(t=>{
                var path = Path.ChangeExtension(Path.Combine(savePath,Path.GetFileName(t.Url)),"pgn");
                Log.Information($"trying to save game in:{path}");
                var fs = new FileStream(path,FileMode.CreateNew,FileAccess.ReadWrite);
                opened.Add(fs);
                var sr = new StreamWriter(fs);
                writers.Add(sr);
                var tsk =  sr.WriteAsync(t.Pgn);
                return tsk;
                });
try
{
    await Task.WhenAll(savingTasks);
    var flushing = writers.Select(u=>u.FlushAsync());
    await Task.WhenAll(flushing);
}
catch(Exception e)
{
     Log.Fatal($"Cannot write to file:{e}");
     throw e;
}
finally
{
     opened.ForEach(s => s.Close());
}

In some steps I'm not convinced I'm doing in the best way, even if the code works just fine. The portion not convincing me is how I handle the closing: I created a group of tasks in the Select, but I had to keep track on the opened file in order to close them ( see finally ), and in a similar way, I had to manage the collection of StreamWriter ( see writers ). This is not convincing me, is there a better approach?

Upvotes: 0

Views: 295

Answers (2)

Johnathan Barclay
Johnathan Barclay

Reputation: 20373

You're over complicating things.

You should use a using block for your FileStream and StreamWriter, which takes care of flushing / closing when they are disposed.

By awaiting WriteAsync rather than returning the Task it generates, will ensure your FileStream and StreamWriter are not disposed of too soon:

var savingTasks = games.Games
    .Select(async t =>
    {
        var path = Path.ChangeExtension(Path.Combine(savePath,Path.GetFileName(t.Url)),"pgn");
        Log.Information($"trying to save game in:{path}");

        using (var fs = new FileStream(path, FileMode.CreateNew, FileAccess.ReadWrite, FileShare.None, bufferSize: 4096, useAsync: true))
        using (var sr = new StreamWriter(fs))
        {
            await sr.WriteAsync(t.Pgn);
        }
    });

try
{
    await Task.WhenAll(savingTasks);
}
catch (Exception e)
{
    Log.Fatal($"Cannot write to file:{e}");
    throw;
}

Upvotes: 3

TiGreX
TiGreX

Reputation: 1816

I will move the FlushAsync to the finally because if an exception jumps during the execution of the tasks they won't be cleared. In addition, I would recommend for cleanliness to do everything in one method like the next: 

var savingTasks = games.Games.Select(t=>ExecuteGameMethod(t));
try
{
     await Task.WhenAll(savingTasks);
}
catch(Exception e)
{
        Log.Fatal($"Cannot write to file:{e}");
        throw;
}


public async Task ExecuteGameMethod(Game game)
{
    var path = Path.ChangeExtension(Path.Combine(savePath,Path.GetFileName(game.Url)),"pgn");
    Log.Information($"trying to save game in:{path}");
    using(var fs = new FileStream(path,FileMode.CreateNew,FileAccess.ReadWrite,bufferSize:4096, isAsync:true ))
    using(var sr = new StreamWriter(fs))
    {
        await sr.WriteAsync(game.Pgn);
        await sr.FlushAsync();
    }

}

Upvotes: 2

Related Questions