Task timer async

Question

I had been still using BackGroundWorker and decided to learn async Task.

My test project is a simple timer that can be canceled.

The target project is to read instruments with a delay for sampling.

Please review for style and anything else you see.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Windows;
using System.Windows.Controls;
using System.Windows.Data;
using System.Windows.Documents;
using System.Windows.Input;
using System.Windows.Media;
using System.Windows.Media.Imaging;
using System.Windows.Navigation;
using System.Windows.Shapes;
using System.Threading;
using System.Diagnostics;
using System.Runtime.CompilerServices;
using System.ComponentModel;
namespace TaskCancel
{
 /// <summary>
 /// Interaction logic for MainWindow.xaml
 /// </summary>
 public partial class MainWindow : Window, INotifyPropertyChanged
 {
 public event PropertyChangedEventHandler PropertyChanged;
 private void NotifyPropertyChanged([CallerMemberName] String propertyName = "")
 {
 if (PropertyChanged != null)
 {
 PropertyChanged(this, new PropertyChangedEventArgs(propertyName));
 }
 }
 public MainWindow()
 {
 this.DataContext = this;
 InitializeComponent();
 btnCancel.IsEnabled = false;
 //start();
 }
 private string sync = "testing";
 public string Async
 {
 get { return sync; }
 set
 {
 if(sync != value)
 {
 sync = value;
 NotifyPropertyChanged("Async");
 }
 }
 }
 // ***Provide a parameter for the CancellationToken. 
 async Task<int> TaskDelayAsync(CancellationToken ct)
 {
 // You might need to slow things down to have a chance to cancel. 
 int i = 0; 
 while (true)
 { 
 i++;
 Debug.WriteLine(i);
 Async = $"AccessTheWebAsync {i}";
 if(ct.IsCancellationRequested)
 {
 Async = $"AccessTheWebAsync cancel";
 break;
 }
 await Task.Delay(1000);
 }
 return i;
 }
 private void cancelButton(object sender, RoutedEventArgs e)
 {
 btnStart.IsEnabled = true;
 btnCancel.IsEnabled = false;
 if (cts != null)
 {
 cts.Cancel();
 }
 }
 CancellationTokenSource cts;
 private async void start()
 {
 // ***Instantiate the CancellationTokenSource. 
 cts = new CancellationTokenSource();
 try
 {
 // ***Send a token to carry the message if cancellation is requested. 
 int contentLength = await TaskDelayAsync(cts.Token);
 Debug.WriteLine($"int contentLength = await TaskDelayAsync(cts.Token); {contentLength}");
 }
 // *** If cancellation is requested, an OperationCanceledException results. 
 catch (OperationCanceledException ex)
 {
 Async = ex.Message;
 }
 catch (Exception ex)
 {
 Async = ex.Message;
 }
 // ***Set the CancellationTokenSource to null when the download is complete. 
 cts = null;
 }
 private void startButton(object sender, RoutedEventArgs e)
 {
 btnStart.IsEnabled = false;
 btnCancel.IsEnabled = true;
 start();
 }
 }
}

.

<Window x:Class="TaskCancel.MainWindow"
 xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
 xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
 xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
 xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
 xmlns:local="clr-namespace:TaskCancel"
 mc:Ignorable="d"
 Title="MainWindow" Height="350" Width="525">
 <Grid>
 <Grid.RowDefinitions>
 <RowDefinition Height="40"/>
 <RowDefinition Height="auto"/>
 </Grid.RowDefinitions>
 <Grid.ColumnDefinitions>
 <ColumnDefinition Width="auto"/>
 <ColumnDefinition Width="auto"/>
 <ColumnDefinition Width="auto"/>
 <ColumnDefinition Width="auto"/>
 </Grid.ColumnDefinitions>
 <TextBlock Grid.Row="1" Grid.Column="0" Text="{Binding Path=Async}" HorizontalAlignment="Left" VerticalAlignment="Center" Width="200" Margin="4"/>
 <Button x:Name="btnStart" Grid.Row="1" Grid.Column="1" Content="Start" Click="startButton" Height="22" Width="40" Margin="4"/>
 <Button x:Name="btnCancel" Grid.Row="1" Grid.Column="2" Content="Cancel" Click="cancelButton" Height="22" Width="40" Margin="4"/>
 </Grid>
</Window>

Answer 1

Initial observation is that you should avoid async void like

private async void start() {
 //...
}

except for event handlers.

Reference Async/Await - Best Practices in Asynchronous Programming

Luckily there is one for the start button.

So start by refactoring start to be proper async

private async Task start() {
 //...
}

and awaiting it in the startButton event handler

private async void startButton(object sender, RoutedEventArgs e) {
 btnStart.IsEnabled = false;
 btnCancel.IsEnabled = true;
 await start();
}

I would personally recommend using a view model. It would actually make reading the code easier and will also separate concerns as the code grows.

public class MainViewModel: INotifyPropertyChanged {
 public event PropertyChangedEventHandler PropertyChanged;
 private void NotifyPropertyChanged([CallerMemberName] String propertyName = "") {
 PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));
 }
 private string sync = "testing";
 public string Async {
 get { return sync; }
 set {
 if(sync != value) {
 sync = value;
 NotifyPropertyChanged("Async");
 }
 }
 }
 private async Task<int> TaskDelayAsync(CancellationToken ct) {
 // You might need to slow things down to have a chance to cancel. 
 int i = 0; 
 while (true) {
 i++;
 Debug.WriteLine(i);
 Async = $"AccessTheWebAsync {i}";
 if(ct.IsCancellationRequested) {
 Async = $"AccessTheWebAsync cancel";
 break;
 }
 await Task.Delay(1000);
 }
 return i;
 }
 private CancellationTokenSource cts;
 public async Task StartAsync() {
 cts = new CancellationTokenSource();
 try {
 int contentLength = await TaskDelayAsync(cts.Token);
 Debug.WriteLine($"int contentLength = await TaskDelayAsync(cts.Token); {contentLength}");
 }
 catch (OperationCanceledException ex) {
 Async = ex.Message;
 } catch (Exception ex) {
 Async = ex.Message;
 }
 cts = null;
 }
 public void Cancel() {
 cts?.Cancel();
 }
}

Note the exposure of StartAsync and Cancel members.

The view now simplifies to

public partial class MainWindow : Window {
 MainViewModel viewModel;
 public MainWindow() {
 InitializeComponent();
 btnCancel.IsEnabled = false;
 viewModel = new MainViewModel();
 this.DataContext = viewModel;
 }
 private void cancelButton(object sender, RoutedEventArgs e) {
 btnStart.IsEnabled = true;
 btnCancel.IsEnabled = false;
 viewModel.Cancel();
 }
 private async void startButton(object sender, RoutedEventArgs e) {
 btnStart.IsEnabled = false;
 btnCancel.IsEnabled = true;
 await viewModel.StartAsync();
 }
}

• Nikita B: This refactoring looks incomplete. Why not go full MVVM and refactor button events into commands?

Answer 2

You're doing all your work in the 'code-behind' rather than a seperate viewmodel class, but for a project this small doing that 'right' would double the amount of code 😀

 //start();
 }
 private string sync = "testing";
 public string Async

I'd recommend putting a new line before your backing field always. As a viewmodel grows having nicely, consistently formatted properties will help quickly browse it to find issues or for adding to it.

Similarly with the startButton method at the end.

if (cts != null)
{
 cts.Cancel();
}

This can (should?) be

cts?.Cancel();

Similarly with your INotifyPropertyChanged implementation, try

PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));

In terms of your xaml, your rows and columns are all auto, why not use stack panels to simplify a little?

<Window x:Class="TaskCancel.MainWindow" 
 ... namespaces and stuff ...>
 <StackPanel>
 <StackPanel Orientation="Horizontal" Margin="0,40,0,0">
 <TextBlock Text="{Binding Path=Async}" HorizontalAlignment="Left" VerticalAlignment="Center" Width="200" Margin="4"/>
 <Button x:Name="btnStart" Click="startButton" Height="22" Width="40" Margin="4">Start</Button>
 <Button x:Name="btnCancel" Click="cancelButton" Height="22" Width="40" Margin="4">Cancel</Button>
 </StackPanel>
 </StackPanel>
</Window>

• paparazzo: Good feedback. I like and columns. As the project grows it seems like I go there.
• Nkosi: I would personally still recommend using a view model. would actually make reading the code easier and will also separate concerns.
• Kelson Ball: Especially since paparazzo mentioned the project growing