I have a function (a event handler in a form) structured as following:
Dim errMsg as String = ""
CheckIfValidUser(..., errMsg)
If errMsg.Length > 0 Then
ShowError(errMsg)
LogError(errMsg)
Return
End If
CheckIfBookAvailable(..., errMsg)
If errMsg.Length > 0 Then
ShowError(errMsg)
LogError(errMsg)
Return
End If
ReserveBook(..., errMsg)
If errMsg.Length > 0 Then
ShowError(errMsg)
LogError(errMsg)
Return
End If
BookReserved = True
I've noticed that most of code is in simular structure so I tried to refactor as following:
Dim errMsg as String = ""
Dim HandleError = Sub()
If errMsg.Length > 0 Then
ShowError(errMsg)
LogError(errMsg)
Return
End If
End Sub
CheckIfValidUser(..., errMsg)
HandleError()
CheckIfBookAvailable(..., errMsg)
HandleError()
ReserveBook(..., errMsg)
HandleError()
BookReserved = True
But it won't work because I need to "return twice" instead of just returning from the nested function! Using goto doesn't work either because the exiting label is out of scope for the nested function.
Is there anyway to do this in .net? I know it is possible to return a boolean from HandleError and branch on that but then it goes back to the same repeated structure again.
If you modify your methods to throw an exception instead of returning a ByRef error message, your code can be rewritten as follows:
Try
CheckIfValidUser(...)
CheckIfBookAvailable(...)
ReserveBook(...)
BookReserved = True
Catch ex As Exception
ShowError(ex.Message)
LogError(ex.Message)
End Try
Usually, catching all exceptions ("Pokemon Exception handling") is considered bad practice, except when doing it at the outermost layer (i.e., the user interface). Since you show an error message (and since you mentioned that this is an event handler in a form), this seems to be the case here.
Another option, if you cannot change the structure of your methods, would be the following:
Dim errMsg as String = ""
CheckIfValidUser(..., errMsg)
If errMsg = "" Then CheckIfBookAvailable(..., errMsg)
If errMsg = "" Then ReserveBook(..., errMsg)
If errMsg <> "" Then
ShowError(errMsg)
LogError(errMsg)
Return
End If
BookReserved = True
Personally, I'd prefer the first option, since that's the more idiomatic to the way things are done in .NET nowadays. Not having to cluster your business logic with technical error handling details is exactly what makes exceptions so powerful.
An option to dealing with exceptions can be to rewrite your Check
subs into functions returning boolean (true
on success, false
if errMsg
contains error) and have a cascade of IF
expressions each calling next function. And have a single piece of error handling code in the end
If CheckIfValidUser(..., errMsg) Then
if CheckIfBookAvailable(..., errMsg) then
ReserveBook(..., errMsg)
End If
End If
If errMsg.Length > 0 Then
ShowError(errMsg)
LogError(errMsg)
Return
End If
BookReserved = True
But I agree it is an old days C style...
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With