HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

WPF - Run an async Task with DispatcherTimer

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
asyncwpfwithdispatchertimertaskrun

Problem

I have a small application that checks some system status and display it details.
I have the following code which is working. But I want to make sure it's optimal:

public partial class MainWindow : Window
{

    // Dictionary to hold info
    Dictionary dataHolder = new Dictionary
    {
        {"status_code", "" },
        {"memory", "" },
        {"process", "" }
    };

    public MainWindow()
    {
        InitializeComponent();
    }

    private void StartBtn_Click(object sender, RoutedEventArgs e)
    {
        StartBtn.IsEnabled = false;

        System.Windows.Threading.DispatcherTimer dispatcherTimer = new System.Windows.Threading.DispatcherTimer();
        dispatcherTimer.Tick += new EventHandler(updateUI);
        dispatcherTimer.Interval = new TimeSpan(0, 5, 0);
        dispatcherTimer.Start();  
    }

    private async void updateUI(object source, EventArgs e)
    {
        await loadInfo();
        StartBtn.IsEnabled = true;

        foreach(KeyValuePair entry in dataHolder)
        {
            // Display in the UI
        }
    }

    private async Task loadInfo()
    {
        int page = 1;
        var keys = new List(dataHolder.Keys);            
        foreach (string key in keys)
        {
            dataHolder[key] = "";
            //await asyncCall(key, page);
        }

        // Better asynchronously and in parallel
        await Task.WhenAll(keys.Select(key => asyncCall(key, page)));
    }

    private async Task asyncCall(string key, int page)
    {
        var htClient = new HttpClient();
        // code to retrieve data...
    }
}


So as I said before, this code is working but I read somewhere that having an async void method is not good practice as in private async void updateUI(object source, EventArgs e). I could change it to private async Task updateUI(object source, EventArgs e) but then I would not be able to do dispatcherTimer.Tick += new EventHandler(await updateUI);

Also how to start counting (Timer) only

Solution

Async void should be avoided, but they are acceptable when in an EventHandler so it's fine.

Why are you copying the keys of the dictionary into a list? Do you want a snapshot at a random point in time?

If you have potentially race conditions, I would change the Dictionary to a ConcurrentDictionary.

public partial class MainWindow : Window
{

    // Dictionary to hold info
    private readonly Dictionary _dataHolder = new Dictionary
    {
        {"status_code", "" },
        {"memory", "" },
        {"process", "" }
    };

    public MainWindow()
    {
        InitializeComponent();
    }

    private async void StartBtn_Click(object sender, RoutedEventArgs e)
    {
        StartBtn.IsEnabled = false;

        while (true) // Your exit condition instead of the true.
        {
            await LoadInfo();

            foreach (var entry in _dataHolder)
            {
                // Display in the UI
            }

            await Task.Delay(TimeSpan.FromMinutes(5));

            //StartBtn.IsEnabled = true;
        }
    }

    private async Task LoadInfo()
    {
        const int page = 1;

        var keys = new List(_dataHolder.Keys);

        foreach (var key in keys)
        {
            _dataHolder[key] = "";
            //await asyncCall(key, page);
        }

        // Better asynchronously and in parallel
        await Task.WhenAll(keys.Select(key => AsyncCall(key, page)));
    }

    private static async Task AsyncCall(string key, int page)
    {
        var htClient = new HttpClient();
        // code to retrieve data...
    }

Code Snippets

public partial class MainWindow : Window
{

    // Dictionary to hold info
    private readonly Dictionary<string, string> _dataHolder = new Dictionary<string, string>
    {
        {"status_code", "" },
        {"memory", "" },
        {"process", "" }
    };

    public MainWindow()
    {
        InitializeComponent();
    }

    private async void StartBtn_Click(object sender, RoutedEventArgs e)
    {
        StartBtn.IsEnabled = false;

        while (true) // Your exit condition instead of the true.
        {
            await LoadInfo();

            foreach (var entry in _dataHolder)
            {
                // Display in the UI
            }

            await Task.Delay(TimeSpan.FromMinutes(5));

            //StartBtn.IsEnabled = true;
        }
    }

    private async Task LoadInfo()
    {
        const int page = 1;

        var keys = new List<string>(_dataHolder.Keys);

        foreach (var key in keys)
        {
            _dataHolder[key] = "";
            //await asyncCall(key, page);
        }

        // Better asynchronously and in parallel
        await Task.WhenAll(keys.Select(key => AsyncCall(key, page)));
    }

    private static async Task AsyncCall(string key, int page)
    {
        var htClient = new HttpClient();
        // code to retrieve data...
    }

Context

StackExchange Code Review Q#133592, answer score: 3

Revisions (0)

No revisions yet.