2
\$\begingroup\$

This code snippet is supposed to utilize async await to resize a photo file into multiple sizes in parallel. I just want to know if I am doing it in the right way. I would love to hear some ideas on how to report resize progress on file like how many of them completed.

public class PhotoProcessor : IPhotoProcessor
{
    public void Resize(string filePath, string dropLocation, List<Size> sizes)
    {
        Parallel.ForEach(sizes, async (size) =>
        {
            var fileName = Path.GetFileName(filePath);
            var result = await Resize(filePath, dropLocation, size);
            Console.WriteLine($"Done {result} Resizing {fileName} to {size}");
        });
    }

    public async Task<bool> Resize(string filePath, string dropLocation, Size size)
    {
        if (!File.Exists(filePath))
        {
            throw new FileNotFoundException($"File {filePath} not found");
        }

        using (var image = new MagickImage(filePath))
        {
            var fileName = Path.GetFileNameWithoutExtension(filePath);
            var extension = Path.GetExtension(filePath);
            var newFileName = $"{fileName}_{size}{extension}";
            var newFilePath = $"{dropLocation}{newFileName}";

            var magickSize = new MagickGeometry(size.W, size.H);
            magickSize.IgnoreAspectRatio = false;
            await Task.Run(() =>
            {
                image.Resize(magickSize);
                image.Write(newFilePath);
            });
        }
        return true;
    }
}
\$\endgroup\$
3
  • 2
    \$\begingroup\$ I would love to hear some ideas on how to report resize progress on file like how many of them completed. That this part of your question seems to be is off-topic because it is not implemented yet. \$\endgroup\$
    – t3chb0t
    Sep 16 '17 at 11:38
  • \$\begingroup\$ @t3chb0t thank you for the edit. Yes that part is not implemented but any suggestions on that are most welcome. \$\endgroup\$
    – iampervaze
    Sep 16 '17 at 11:51
  • 3
    \$\begingroup\$ You are reading the image multiple times. It is one hard drive with one set of RW heads. Parallel is not gong to make the HW faster. I bet a simple single read and multiple Resize and Write would be faster. \$\endgroup\$
    – paparazzo
    Sep 16 '17 at 16:00
4
\$\begingroup\$
    public void Resize(string filePath, string dropLocation, List<Size> sizes)

Why List<>? Following the principle of coding to the interface rather than the implementation, this should probably be IList<> or even IReadOnlyList<>.


        Parallel.ForEach(sizes, async (size) =>
        {
            var fileName = Path.GetFileName(filePath);
            var result = await Resize(filePath, dropLocation, size);
            Console.WriteLine($"Done {result} Resizing {fileName} to {size}");
        });

Why async/await inside Parallel.ForEach? If each size is handled on a different thread, there's nothing to be gained by using await.


            var newFilePath = $"{dropLocation}{newFileName}";

Path.Combine exists for a reason: using that rather than manual concatenation will give more robust code.


            await Task.Run(() =>
            {
                image.Resize(magickSize);
                image.Write(newFilePath);
            });

Pretend for a moment that we're not using Parallel.ForEach. The benefit of async/await with something like image processing is that you can keep the CPU busy even when one of the coroutines is blocked on I/O. But half of the I/O has already happened (maybe more if you're always scaling down rather than up). If you want to make an async version, the image load should be async.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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