Broadly speaking, the validation of input data to a procedure should
be within the procedure itself, and should execute before any other
code. However, it might be the case that several different procedures
have the same validation needs. For example, you may need to validate
whether a server share is available, and do this validation in a
number of independent procedures. In this case, you would probably
want to move the actual data validation to its own proc and call that
proc whenever the validation is required. If multiple procedures need
the same validation and that validation is complex, then you should
put it in its own proc that returns a Boolean indicating whether the
data is valid.

There is no benefit, and there is some (very small) cost, to moving
data validation code out of the proc that needs it to another proc.
Taking it out to another proc just for the sake of taking it out buys
you nothing.

Whoever told you that a single proc should do a single thing was
correct and you would do well to follow that recommendation. However,
in this context, I would consider any input validation to be a
subordinate task of the primary task carried out by the proc.

Cordially,
Chip Pearson
Microsoft MVP 1998 - 2010
Pearson Software Consulting, LLC
www.cpearson.com
[email on web site]



On Thu, 4 Feb 2010 09:59:34 -0800 (PST), deltaquattro
wrote:

Hello,

taking the cue from the previous thread I opened ("Error handling in
VBA"), I would like to ask you if in your opinion it's usually better
to check input data to a subroutine, inside the subroutine, or to move
each check to a distinct subroutine. As an example, I'll post again my
Interp subroutine, this time with some modifications added by Peter T:

Public Function Interp2(xArr() As Double, yArr() As Double, _
x As Double) As Double
Dim i As Long

' this is the part ...

If ((x < xArr(LBound(xArr))) Or (x xArr(UBound(xArr)))) Then
'MsgBox "Interp2: x is out of bound" & "X = " & x

'.... I am considering moving outside the subroutine

Else

For i = LBound(xArr) To UBound(xArr)
If xArr(i) = x Then
Interp2 = yArr(i)
Exit For
ElseIf xArr(i) x Then
Interp2 = yArr(i - 1) + (x - xArr(i - 1)) / _
(xArr(i) - xArr(i - 1)) * (yArr(i) - yArr(i
- 1))
Exit For
End If
Next i
End If
End Function

From one point of view, I would say yes, because a single subroutine
should just do a single thing, as someone on this ng recently
reminded me (don't remember who). On the other hand, if one follows
this principle too strictly, wouldn't he end writing "unsafe" code? If
I take out the check from Interp2 and move it to another subroutine
which the caller has to call before Interp2, I risk forgetting to add
the call to the checker subroutine when I reuse Interp2 in another
code. Sure, I could place the call to the checking sub inside Interp2:
however, this way I end up having a code with the same functionalities
as before, but with the added slowdown due to the call to the checking
subroutine.

Best Regards

Sergio