WPF - Run an async Task with DispatcherTimer

Question

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<string, int> dataHolder = new Dictionary<string, string>
 {
 {"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<string, string> entry in dataHolder)
 {
 // Display in the UI
 }
 }
 private async Task loadInfo()
 {
 int page = 1;
 var keys = new List<string>(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 after the previous process is done?

Thank you

EDIT: I am using the DispatcherTimer because I want the application to retrieve the status information and update the UI every 5 minutes.

• pacmaninbw: Welcome to code review, I hope you get some good answers.

Answer 1

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<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...
 }

• idris: I want the app to update the status information every 5 minutes.
• Stefano d'Antonio: If you click on the start button again, what are you expecting to happen?
• idris: When the application is launched, nothing happens (UI is empty). Then when you click on start button, it updates the information immediately then 5 minutes later, it refreshes and again 5 min, a new refresh and so on
• Stefano d'Antonio: Why were you enabling the start button again?
• idris: This is just the first part. Eventually I want the user to enter a parameter in a textbox and launch the cycle of status check. But then he can change the parameter and start over.