CODEWITHSUNDEEP
c#asynchronousasync-awaittask
Martin Niederl
Martin Niederl

Reputation: 759

Calling 2 methods asynchronously

I'm not sure if my program does exactly what i want or if it could be coded better. Target would be to call the 2 Methods (CreateHeaderAsync, CombineFilesAsync) and let them work parallel.

I don't think this is the right or a good way to write those ...Async() methods because that looks pretty unclean.

And couldn't i combine the first 4 lines in my CreateFile method to reduce unimportant code and make it more readable?

public void CreateFile(string path)
{
    Task<byte[]> headerTask = CreateHeaderAsync();
    Task<byte[]> filesTask = CombineFilesAsync();
    byte[] header = headerTask.Result;
    byte[] files = filesTask.Result;

    byte[] combined = new byte[header.Length + files.Length];

    Buffer.BlockCopy(header, 0, combined, 0, header.Length);
    Buffer.BlockCopy(files, 0, combined, header.Length, files.Length);

    Task.Factory.StartNew(() => File.WriteAllBytes(path, combined));
}

private Task<byte[]> CreateHeaderAsync()
{
    return Task.Factory.StartNew(() =>
    {
        StringBuilder sb = new StringBuilder();
        int position = 0;
        foreach (ByteFile file in _files)
        {
            sb.Append(file + "?" + position + Environment.NewLine);
            position += file.Length;
        }
        return Encoding.UTF8.GetBytes(sb + "--header_end--" + Environment.NewLine);
    });
}

private Task<byte[]> CombineFilesAsync()
{
    return Task.Factory.StartNew(() =>
    {
        ByteFile[] arrays = _files.ToArray();

        byte[] rv = new byte[arrays.Sum(a => a.Length)];
        int offset = 0;
        foreach (ByteFile t in arrays)
        {
            var array = Encryption.EncryptBytes(t.Content, "password");

            Buffer.BlockCopy(array, 0, rv, offset, array.Length);
            offset += array.Length;
        }
        return rv;
    });
}

Upvotes: 2

Views: 141

Answers (3)

user6996876
user6996876

Reputation:

Target would be to call the 2 Methods (CreateHeaderAsync, CombineFilesAsync) and let them work parallel.

Then try Task.WhenAll to asynchronously await their completion. 

  public async Task CreateFile(string path)
    {
        Task<byte[]> headerTask = CreateHeaderAsync();
        Task<byte[]> filesTask = CombineFilesAsync();

        var allResults = await Task.WhenAll(headerTask, filesTask);
        byte[] header = allResults[0];
        byte[] files = allResults[1];

Reply to the comment

Couldn't he also just do byte[] header = await headerTask; byte[] files = await filesTask; Does it matter that he waits for one task even though the other may complete before it?

Ok. This is true. The only thing is that two await could resume the caller once more. So I would prefer a single final callback.

Upvotes: 2

Tomas Chabada
Tomas Chabada

Reputation: 3019

I would go for async/await pattern. Using Task.Result causes calling thread to be blocked, which in some cases can lead to deadlock.

If you do not want to play with task creation options and you do not want to be able to cancel tasks I recommend you to create tasks using Task.Run(() => {}). In most cases that is just what you need.

So change your async methods signature to public async ... and then instead of calling task.Result just await that tasks: 

var taskResult = await MyMethodAsync();

Upvotes: 0

P. Fouilloux
P. Fouilloux

Reputation: 21

If you wish them to run in parallel, you could use WhenAll.

https://msdn.microsoft.com/en-us/library/hh194874(v=vs.110).aspx

Something like this:

public void CreateFile(string path)
{
    Task<byte[]>[] tasks = new [] { CreateHeaderAsync(), CombineFilesAsync() };

    var resultSet = Task.WhenAll(tasks); // create an aggregate task
    try 
    {
        resultSet.Wait(); // await the results
    }
    catch (AggregateException)
    {
        // handle exceptions in the tasks here
    }

    if (resultSet.Status == TaskStatus.RanToCompletion) 
    {
         // get your results
         byte[] header = resultSet.Result[0]; 
         byte[] files = resultSet.Result[1];

         byte[] combined = new byte[header.Length + files.Length];
         Buffer.BlockCopy(header, 0, combined, 0, header.Length);
         Buffer.BlockCopy(files, 0, combined, header.Length, files.Length);

         Task.Factory.StartNew(() => File.WriteAllBytes(path, combined));
    } 
}

Also, in this case, I would find Task.Run() to be a little more readable than Task.Factory.StartNew().

https://blogs.msdn.microsoft.com/pfxteam/2011/10/24/task-run-vs-task-factory-startnew/

It allows you to make the CreateHeaderAsync and CombineFilesAsync a bit smaller by concentrating all the Tasks in the CreateFile method.

public void CreateFile(string path)
{
    Task<byte[]>[] tasks = new [] { 
          Task.Run((byte[])CreateHeaderAsync), 
          Task.Run((byte[])CombineFilesAsync)};

    // rest of the method 
}

private byte[] CombineFilesAsync()
{
     ByteFile[] arrays = _files.ToArray();

     byte[] rv = new byte[arrays.Sum(a => a.Length)];
     int offset = 0;
     foreach (ByteFile t in arrays)
     {
        var array = Encryption.EncryptBytes(t.Content, "password");

        Buffer.BlockCopy(array, 0, rv, offset, array.Length);
        offset += array.Length;
     }
     return rv;
}

Wrote this a bit fast and without checking, apologies if any weird mistakes snuck in there :)

Upvotes: 0

Related Questions