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();
    }
}

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>