Message in the Caller or the Function

[fredlo2010]

Hello,

I placed this question here because I am not sure it would anywhere else, admins feel free to move it if needed.

I was writing some code today and I noticed something I have been doing for a while but now I don't know if this is the best way to do it; or if there is such a thing as "the best way to do it"

If my function is supposed to return a boolean then I will place a message stating that it failed. In other words I have delegated the message to be handled by the function rather than the main code.

Any thoughts ?

[Bob Phillips]

I don't know about a best or proper way, but I don't delegate the message. I like my function to return the status and then the caller decides what action to take.

[fredlo2010]

Thanks xld,

But what about a function that is called several times; like a find function. I would have to type the same message all the time.


In the example bellow I would've had to type the same message 3 times.


Option Explicit

Sub MainRunner()

Dim rApple As Range
Dim rPine As Range
Dim rOrange As Range

Set rApple = GetCellbyCriteria("Apple")


If Not rApple Is Nothing Then
rApple.Interior.ColorIndex = 3
End If


' =======================
' Some other lengthy code
' =======================

If Not rPine Is Nothing Then
rPine.Interior.ColorIndex = 3
End If


' =======================
' Some other lengthy code
' =======================

If Not rOrange Is Nothing Then
rOrange.Interior.ColorIndex = 3
End If

End Sub






Public Function GetCellbyCriteria(ByVal strCriteria As String) As Range


Dim ret As Range

Set ret = Cells.Find(strCriteria)


If Not ret Is Nothing Then
Set GetCellbyCriteria = ret
Else
MsgBox "The criteria " & strCriteria & " counld not be found."
End If

' Clean up
Set ret = Nothing

End Function



Thank you :)

[Bob Phillips]

You would, but my argument is that it is more important to have code clarity, logical flow and so on than save the few seconds that the repeated typing would entail.

I am adamantly against shortcutting in code, using stuff like [A1], not stating the property because it is the default, etc., etc. You type it once, you look at it (try toi understand it) dozens of time.

[Zack Barresse]

I hear you. It can often seem like a shortcut. While I've done it, I'm with Bob in that I think it's not necessarily the best way to structure your code. One way I've handled this before is to just make an Optional variable in the called routine, usually titled something deft and clever like Optional ByVal MessageUser As Boolean = False or something to the like. It allowed me to build in a message into the functions themselves and when called they don't message anything unless I specify it to. More and more I found I would never use the variable though, as I never really wanted to handle it inside of a called procedure, but in the superior wrapper code, because I can handle it more effectively there.

Another option would be to build out some sort of class, which would allow you to contain custom procedures as well as properties (although you could just as easily create a property in a standard module). In the end I think a logical flow of code is most important. While it could be argued one way is better than the other, if you're the one using the code, I think it boils down to what works best for you. I won't code the same way you will, or Bob, but it doesn't make me (or you) wrong necessarily. Part of the beauty of programming. :)

[fredlo2010]

Thanks a lot for the response.

I totally get it; and I also have the optional parameter for the message :)

This is my situation. I want to be able to show those error messages so the user knows what's going on and what's missing, but at the same time I want the code to continue running. It's like a flow with checks and stops that allow the users to move forward as the items get checked; but I want to guide the user so they can easily locate the errors (Differences).

So far I have delegated the message to a four functions; GetColumnbyCriteria, GetRowbyCriteria and their Date counter parties. And I call this functions about 20-30 times. Now the user has other checks so they will never get 30 message boxes about missing rows (If they did then we should revisit their training :))

I guess I will keep it the way it is so far and use it only under extreme circumstances and after I have evaluated all the possibilities carefully.


Thanks a lot for the comments both of you.

[Zack Barresse]

Sounds like possibly a good candidate for a message/error class module. I've done similar things, where each item gets logged and put into a Collection. Then when you're done just test the Collection to see if it contains anything, and if so iterate through each item and do what you want with it. The Collection would need to be a non-private variable in a standard module, so you could access it wherever you are in your process, regardless of the module. Sounds doable though.

[Kyle234]

Why wouldn't you just raise an error? The calling code can then take action as appropriate

Example:

Public Function GetCellbyCriteria(ByVal strCriteria As String) As Range

Dim ret As Range

Set ret = Cells.Find(strCriteria)

If Not ret Is Nothing Then
Set GetCellbyCriteria = ret
Else
Err.Raise vbObjectError + 1000, Description:="The criteria " & strCriteria & " counld not be found."
End If

End Function


Sub test()


On Error GoTo handler

GetCellbyCriteria ("sdfsdfsdfsdfsdfsd")
Exit Sub

handler:
MsgBox Err.Description


End Sub

[fredlo2010]

Thanks for the reply Kyle.

This is a very goo idea on how to handle this. The only problem with it is that it will add complexity to the code. We are talking here of about 2000 lines of code that call this function about 20 times. Rerouting and trying to keep the flow will be a little but hard I think.

:) :) :)

[Bob Phillips]

I use Err.Raise a lot in my code, I don't think it adds complexity, in fact it can reduce the complexity if you are liable to have a raft of nested IFs.

I have a set of error enum values, including one called Bypass, and where I just want to exit I raise a Bypass error, anbd then trap that in the error handler, like this


Public Function ShowErrBypas()

On Error GoTo errHandler

'some test
If something Then

do_something
If return_code = Abandon Then Err.Raise appErrors.Bypass
End If
If something_else Then

do_something_else
If return_code = AbandonThen Err.Raise appErrors.Bypass
End If
If something_else_again Then

do_something_else_again
If return_code = AbandonThen Err.Raise appErrors.Bypass
End If
'and so on

'do some more

' and some more

mod_exit:
Exit Function

errHandler:
If Err.Number = appErrors.Bypass Then Resume mod_exit
'normal error handling
End Function
#

Without raising a pseudo-error, I would have to test each following piece of code whether I continue or not.

[Kyle234]

How difficult it is to implement depends on what exactly you want to do should you encounter the error, you could do something like:



Sub test()


Dim errors As Collection
Dim x As Long
Dim er As Variant


On Error GoTo handler

For x = 1 To 5
GetCellbyCriteria ("sdfsdfsdfsdfsdfsd")
Next x


If Not errors Is Nothing Then
Debug.Print "Number of Errors Encountered:", errors.Count
Debug.Print "Error Summary:"
For Each er In errors
Debug.Print er
Next er
End If

Exit Sub



handler:
If errors Is Nothing Then Set errors = New Collection
errors.Add Err.Description
Err.Clear
Resume Next




End Sub