7
\$\begingroup\$

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>
\$\endgroup\$
10
\$\begingroup\$

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();
    }
}
\$\endgroup\$
  • \$\begingroup\$ This refactoring looks incomplete. Why not go full MVVM and refactor button events into commands? \$\endgroup\$ – Nikita B May 17 '18 at 7:16
6
\$\begingroup\$

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>
\$\endgroup\$
  • \$\begingroup\$ Good feedback. I like and columns. As the project grows it seems like I go there. \$\endgroup\$ – paparazzo May 16 '18 at 23:45
  • 2
    \$\begingroup\$ I would personally still recommend using a view model. would actually make reading the code easier and will also separate concerns. \$\endgroup\$ – Nkosi May 16 '18 at 23:50
  • \$\begingroup\$ Especially since paparazzo mentioned the project growing \$\endgroup\$ – Kelson Ball May 16 '18 at 23:53

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.