Fill ObservableCollection with results of multiple async calls without waiting

Question

In my Prism module in ViewModel class in OnNavigatedTo method

I would like to fill an ObservableCollection with results of multiple async calls without waiting for all calls to complete.

I am using answer from this question: How to hydrate a Dictionary with the results of async calls?

The following code is a cleaned-up version of my real code:

My status class:

public class Status
{
    public string ipAddress;
    public string status;
}

My view model:

using Prism.Mvvm;
using Prism.Regions;

public class StatusViewModel : BindableBase, INavigationAware
{
    ObservableCollection<Status> statusCollection = new ObservableCollection<Status>();
    HttpClient httpClient = new HttpClient();
    Ping ping = new Ping();
    List<string> ipAddressList = new List<string>();

    public void OnNavigatedTo(NavigationContext navigationContext)
    {
        GetEveryStatus();
    }

    public void GetEveryIp() // this is not important, works ok
    {
        var addressBytes = Dns.GetHostEntry(Dns.GetHostName()).AddressList.FirstOrDefault(ip => ip.AddressFamily == AddressFamily.InterNetwork).GetAddressBytes();
        for (byte i = 1; i < 255; ++i)
        {
            addressBytes[3] = i;
            string ipAddress = new IPAddress(addressBytes).ToString();
            if (ping.Send(ipAddress, 10).Status == IPStatus.Success)
            {
                ipAddressList.Add(ipAddress);
            }
        }
    }

    public void GetEveryStatus() // this is important, here is the problem
    {
        GetEveryIp();
        var task = GetStatusArray(ipAddressList);
        statusCollection.AddRange(task.Result);
    }

    // solution from stackoverflow, but it throws exception
    public async Task<Status[]> GetStatusArray(List<string> ipAddressList)
    {
        Status[] statusArray = await Task.WhenAll(
            ipAddressList.Select(
                async ipAddress => new Status(
                    ipAddress,
                    await httpClient.GetStringAsync("http://" + ipAddress + ":8080" + "/status")
                    )
                )
            );

        return statusArray;
    }
}

but that didn't work because GetStringAsync can throw an exception, so I changed it to this:

    public void GetEveryStatus() // this is important, here is the problem
    {
        GetEveryIp();
        foreach (string ipAddress in ipAddressList)
        {
            try
            {
                var task = httpClient.GetStringAsync("http://" + ipAddress + ":8080" + "/status");
                statusCollection.Add(new Status(ipAddress, task.Result));
            }
            catch (Exception)
            {
            }
        }
    }

but it still doesn't work.

What is the right way to do this? Thank you!

read documentation of `Task.Result` and tell us what is a point of `task.Wait(10);` before `task.Result` ? — Selvin, Nov 08 '18 at 13:38
Why don't you make GetEveryStatus async? And use await instead of task.Wait() — Access Denied, Nov 08 '18 at 13:39
@Selvin "A TimeSpan that represents the number of milliseconds to wait, or a TimeSpan that represents -1 milliseconds to wait indefinitely." am i using this wrong? i want to wait for 10ms for the GetStringAsync to complete? — Jinjinov, Nov 08 '18 at 13:41
documentation of `Task.Result` (look at Remarks section) not `Task.Wait` — Selvin, Nov 08 '18 at 13:41
@AccessDenied OnNavigatedTo is not async, it is the implementation of the INavigationAware — Jinjinov, Nov 08 '18 at 13:42
You can make it async, it does not change the interface, I see no problem. — Access Denied, Nov 08 '18 at 13:43
If it won't work just use taskfactory.startnew to run method in threadpool, or you can also try NotifyTask. — Access Denied, Nov 08 '18 at 13:52
What exactly is your issue? That you will be blocked until all calls have completed? — mm8, Nov 08 '18 at 13:59
@mm8 the issue is that OP calls async code synchrouslt and all code is blocked in onnavigatedto. — Access Denied, Nov 08 '18 at 14:03
That's because neither `GetEveryStatus` nor `GetEveryIp` are asynchronous. So execute them on a background thread? — mm8, Nov 08 '18 at 14:05
@mm8 Making OnNavigatedTo async and awaiting task will be enough I think — Access Denied, Nov 08 '18 at 14:08
@JinJi async is a keyword which makes compiler interpreting await keyword in method. This was done for backward compatibility not to break old code which could have await variables. — Access Denied, Nov 09 '18 at 02:01

Jinjinov · Accepted Answer · 2018-11-09T12:35:23.067

Thanks to @AccessDenied for explaining the role of async in interface implementation.

Thanks to @Selvin for explaining Task.Result and Task.Wait.

If anyone is interesed in the final solution, here it is:

PositioningModule is a hardware device, this class has nothing to do with Prism.Modularity.IModule

public class PositioningModule
{
    public string IpAddress { get; set; }
    public PositioningModuleStatus PositioningModuleStatus { get; set; }

    public PositioningModule(string ipAddress, PositioningModuleStatus positioningModuleStatus)
    {
        IpAddress = ipAddress;
        PositioningModuleStatus = positioningModuleStatus;
    }
}

The ViewModel:

I had to use BindingOperations.EnableCollectionSynchronization and lock on the ObservableCollection. This is the main reason why it didn't work async before!

Changing OnNavigatedTo to async blocked the UI, so I used Task.Run().

using Prism.Mvvm;
using Prism.Regions;

public class DomePositioningViewModel : BindableBase, INavigationAware
{
    ObservableCollection<PositioningModule> _positioningModuleCollection = new ObservableCollection<PositioningModule>();
    readonly object _lock = new object();

    DomePositioningModel _domePositioningModel = new DomePositioningModel();

    public DomePositioningViewModel()
    {
        BindingOperations.EnableCollectionSynchronization(_positioningModuleCollection, _lock);
    }

    public /* async */ void OnNavigatedTo(NavigationContext navigationContext)
    {
        //await _domePositioningModel.ScanForModulesAsync(AddModule); - this blocks the UI

        Task.Run(() => _domePositioningModel.ScanForModulesAsync(AddModule));
    }

    private void AddModule(PositioningModule module)
    {
        lock (_lock)
        {
            _positioningModuleCollection.Add(module);
        }
    }
}

The Model:

I changed Send to SendPingAsync and I had to use new Ping() instead of ping.

Using Select instead of foreach to make the calls parallel made everything much faster!

#define PARALLEL

public class DomePositioningModel
{
    private readonly HttpClient _httpClient = new HttpClient();

    public DomePositioningModel()
    {
        _httpClient.Timeout = TimeSpan.FromMilliseconds(50);
    }

    public async Task ScanForModulesAsync(Action<PositioningModule> AddModule)
    {
        List<string> ipAddressList = new List<string>();

        var addressBytes = Dns.GetHostEntry(Dns.GetHostName()).AddressList.FirstOrDefault(ip => ip.AddressFamily == AddressFamily.InterNetwork).GetAddressBytes();

        for (addressBytes[3] = 1; addressBytes[3] < 255; ++addressBytes[3])
        {
            ipAddressList.Add(new IPAddress(addressBytes).ToString());
        }

        //Ping ping = new Ping(); - this behaves strangely, use "new Ping()" instead of "ping"

#if PARALLEL
        var tasks = ipAddressList.Select(async ipAddress => // much faster
#else
        foreach (string ipAddress in ipAddressList) // much slower
#endif
        {
            PingReply pingReply = await new Ping().SendPingAsync(ipAddress, 10); // use "new Ping()" instead of "ping"

            if (pingReply.Status == IPStatus.Success)
            {
                try
                {
                    string status = await _httpClient.GetStringAsync("http://" + ipAddress + ":8080" + "/status");

                    if (Enum.TryParse(status, true, out PositioningModuleStatus positioningModuleStatus))
                    {
                        AddModule?.Invoke(new PositioningModule(ipAddress, positioningModuleStatus));
                    }
                }
                catch (TaskCanceledException) // timeout
                {
                }
                catch (HttpRequestException) // could not reach IP
                {
                }
                catch (Exception ex)
                {
                    System.Windows.MessageBox.Show(ex.Message);
                }
            }
        }
#if PARALLEL
        );
        await Task.WhenAll(tasks);
#endif
    }
}

It didn't benchmark it because the difference is so obvious - about 0.5 sec instead of 14 sec!

Fill ObservableCollection with results of multiple async calls without waiting

1 Answers1