4

I've made the code below work and it is mostly non-blocking except where the process.start code is.

However, my question is, in my winforms application is this the best way to implement the use of async/await to prevent blocking?

Having googled a fair bit to get this far, I'm not terribly clear on what better way I could have done this code in this context, however I'd be grateful for any pointers.

At the UI Layer:

Private Async Sub getcontacts()
    Dim t = ImportMUSContactsAsync()
    Await t
    If t.IsCompleted Then
        BuildLookups()
        MainForm.HideWait()
    End If
End Sub

At the Datalayer:

Private ImportFiles As New List(Of MUSFile) 'Elided full definition
Public Async Function ImportMUSContactsAsync() As Task(Of Boolean)
        Await Task.Run(Sub() DeleteExistingCSV())
        Await Task.Run(Sub() ConvertTPSToCSV())
        Await Task.Run(Sub() ReadCSVFiles())
        Return True
End Function

Called Sub routines

Private Sub ConvertTPSToCSV()
    Dim CSVFolder = New DirectoryInfo(CSVPmanPath)
    If Not CSVFolder.Exists Then CSVFolder.Create()
     For Each f In ImportFiles
       If File.Exists(f.TPSFilename) Then
       Dim p = New ProcessStartInfo($"{PathToConverter}", $" csv {f.TPSFilename} {f.CSVFilename}")
       Process.Start(p).WaitForExit()
     End If
    Next
End Sub

Private Sub ReadCSVFiles()
For Each f In ImportFiles
    If File.Exists(f.CSVFilename) Then ReadSingleCSV(f)
Next
End Sub

Private Sub ReadSingleCSV(f As MUSFile)
Using textReader As New StreamReader(f.CSVFilename)
    Using csv As New CsvReader(textReader)
     CSVMapToContact(csv, f)
    End Using
     End Using 
End Sub
Deduplicator
  • 8,591
  • 5
  • 31
  • 50
Richard
  • 211
  • 1
  • 7
  • 1
    You (probably) don't need to wait for each Process to finish before starting the next one. [see here](https://stackoverflow.com/questions/10788982/is-there-any-async-equivalent-of-process-start) – Caleth Jun 16 '17 at 13:57

2 Answers2

1

For correctly implementing asynchronous methods you should make all your methods which "talking" to external resources "really" asynchronous.

Wrapping synchronous methods with new thread(Task.Run(...)) will not provide all benefits you get from async-await features.
By wrapping synchronous methods with new thread you creating thread which do nothing - only waiting for response from external resource, where async-await provide possibility to do this in one thread without blocking it.

For example starting new process and waiting it for exit can be rewritten it with manually created Task

Public Module ConverterModule
    Private Const PATH_TO_CONVERTER AS String = "yourPath"
    Public Function ConvertAsync(arguments As String) As Task
        Dim taskSource As New TaskCompletionSource(Of Object)()

        Dim process = New Process With 
        { 
            .StartInfo = New ProcessStartInfo With
            {
                .FileName = PATH_TO_CONVERTER,
                .Arguments = arguments
            }
        }

        Dim exitHandler As Action(Of Object, EventArgs) = 
            Sub (sender, args)
                taskSource.TrySetResult(Nothing)
                process.Dispose()
            End Sub            
        AddHandler processInfo.Exited, exitHandler

        process.Start()

        Return taskSource.Task 
    End Function
End Module

Then use it in ConvertTPSToCSV method

Private Function ConvertTPSToCSVAsync() As Task
    Dim CSVFolder = New DirectoryInfo(CSVPmanPath)
    If Not CSVFolder.Exists Then CSVFolder.Create()

    Dim tasks = New List(Of Task)()
    For Each file In ImportFiles
        Dim args As String = $" csv {file.TPSFilename} {file.CSVFilename}"
        Dim fileTask = ConverterModule.ConvertAsync(args)
        tasks.Add(fileTask)
    Next

    Return Task.WhenAll(tasks)
End Sub

For reading/writing files .NET provide built-in methods, for example StreamReader.ReadLineAsync or SreamReader.ReadToEndAsync which you can use for reading files in ReadSingleCSV method.
I don't know about CsvReader class does it supports ..Async methods, if not, then maybe you can read whole file by yourself and pass whole already retrieved data to CsvReader.

Then when you have "correctly" implemented async-await methods you can "combine" them in "Datalayer" method

Private ImportFiles As New List(Of MUSFile) 'Elided full definition
Public Async Function ImportMUSContactsAsync() As Task
    Await DeleteExistingCSVAsync()
    Await ConvertTPSToCSVAsync()
    Await ReadCSVFilesAsync()
End Function

And you can little bid simplify your UI method

Private Async Sub GetContactsAsync()
    Await ImportMUSContactsAsync()

    BuildLookups()
    MainForm.HideWait()
End Sub
Fabio
  • 3,086
  • 1
  • 17
  • 25
  • Thank you Fabio, I'll investigate this when I'm reworking the datalayer and let future followers know what worked in my project. Very much obliged to you. – Richard Jun 19 '17 at 13:34
  • Creating new Task is not always equal to creating new Thread. This is completely different. See https://blogs.msdn.microsoft.com/pfxteam/2009/06/02/the-nature-of-taskcompletionsourcetresult/ – eriawan Jun 29 '17 at 07:35
  • @eriawan - true - but my point was that creating new Task by using `Task.Run` will use other threads for executing a function. Where "correctly" implemented asynchronous methods can be executed on one one thread. – Fabio Jun 29 '17 at 09:04
-1

I'm not sure about what would be correct or not based on the question, but you can use BackgroundWorker. Visual Studio allows you to drop them in as form elements in the editor as well. This will go off and run tasks asynchronously and you can perform updates to the UI via invocation, i.e. form.Invoke(...). You could also make each form method aware of whether it needs to invoke itself/return control flow and then simply call the function.

An example of such a function would be:

Public Sub Progress(Value As Integer, Type As ProgressType)
    Dim control As ProgressBar = Nothing
    Dim label As Label = Nothing
    Select Type
        Case ProgressType.Process
            control = pbProcess
            label = lblProcessProgress
        Case ProgressType.Stage
            control = pbStage
            label = lblStageProgress
        Case ProgressType.Both
            Progress(Value, ProgressType.Stage)
            Progress(Value, ProgressType.Process)
            Exit Sub
        Case Else
            Throw New NotImplementedException()
    End Select

    If control.InvokeRequired Then
        control.Invoke(New Action(Sub() Progress(Value, Type)))
        Exit Sub
    End If

    control.Value = Value
    If Value = 0 Then
        label.Text = "---"
    ElseIf Value >= 100 Then
        label.Text = Value.ToString()
    Else
        label.Text = Value & "%"
    End If
End Sub

BackgroundWorker naturally supports cancellation and with not a lot of glue code, you can write pause/resume functionality. The task that you're running only has to implement checks for cancellation/pause.

Because this entire thing goes off and executes asynchronously, you don't have to implement any asynchronous code yourself. It can all be blocking code. However if you want to keep the updates clear, you can still use it asynchronously and then update in a clear single spot with the result.

Deduplicator
  • 8,591
  • 5
  • 31
  • 50
Joel
  • 1,169
  • 8
  • 12
  • Thank you. I'll have to investigate where this will help most, probably in the forms calling routine for the import. The data layer had no reference to gui libraries at all. – Richard Jun 18 '17 at 05:10
  • `BackgroundWorker` will create new thread for "background" task, so it will not differ much from `Task.Run` used by OP. – Fabio Jun 18 '17 at 13:07
  • And seems little bid strange that you using "workaround" `form.Invoke` for updating UI, where `BackgroundWorker` was designed exact for this purpose and have method `BackgroundWorker.ReportProgress(...)` which will raise `ProgressChanged` event and that handlers will be executed on UI thread - where you can "correctly" update controls with new values – Fabio Jun 18 '17 at 13:11