SubRoutine Within Procedure: Good Practice?

[PeteCresswell]

I've never done it before, but the idea of including a subroutine that
is used only by a certain procedure within that procedure instead of
declaring it separately as a local procedure is starting to appeal to
me.

e.g. below. Pretend we are saving something called a "Security",
which has many child tables.
Also pretend that to process a child table we need to open some sort
of recordset and interate through it - same recordset for all child
tables.

Instead of declaring a separate Function to open said RecordSet and
return a pointer to it, we might just put the code in the using
procedure as below.

Seems like it adds a little bit to the ease of understanding in that
it's now obvious that nobody else uses that code.

It also seems to enhance readability by putting all the code in one
window.

But is it considered good practice?
----------------------------------------------------------------
Sub Security_Save(byVal theSecurityID As Long)
On Error GoTo Security_Save_Err

Dim myRS As DAO.RecordSet
Dim myQuery As DAO.QueryDef
Dim listCreated As Boolean

If mDirty_Security = True then
(do some stuff to save the Security record)
End If

If mDirty_ResetSchedule = True Then
If listCreated = False Then
createList
Else
myRS.MoveFirst
End If

(do some stuff the save changes to reset
schedule that uses the list
End If

If mDirty_PaymentSchedule = True Then
If listCreated = False Then
createList
Else
myRS.MoveFirst
End If

(do some stuff the save changes to Payment
schedule that uses the list)
End If


security_Save_xit:
On Error Resume Next
myQuery.Close
Set myQuery = Nothing
myRS.Close
set myRS = Nothing
Exit Sub

security_Save_err:
MsgBox: Error$,vbCritical, "Trouble In River City"
Resume Security_Save_xit

createList:
Set myQuery = CurrentDB.QueryDefs("qrySecurityList")
With myQuery
.Parameters("theSecurityID") = theSecurityID
Set myRS = .OpenRecordSet(dbOpenDynaSet)
End With
End Sub
----------------------------------------------------------------

 

[Dirk Goldgar]

I've never done it before, but the idea of including a subroutine that
is used only by a certain procedure within that procedure instead of
declaring it separately as a local procedure is starting to appeal to
me.

e.g. below. Pretend we are saving something called a "Security",
which has many child tables.
Also pretend that to process a child table we need to open some sort
of recordset and interate through it - same recordset for all child
tables.

Instead of declaring a separate Function to open said RecordSet and
return a pointer to it, we might just put the code in the using
procedure as below.

Seems like it adds a little bit to the ease of understanding in that
it's now obvious that nobody else uses that code.

It also seems to enhance readability by putting all the code in one
window.

But is it considered good practice?
----------------------------------------------------------------
Sub Security_Save(byVal theSecurityID As Long)
On Error GoTo Security_Save_Err

Dim myRS As DAO.RecordSet
Dim myQuery As DAO.QueryDef
Dim listCreated As Boolean

If mDirty_Security = True then
(do some stuff to save the Security record)
End If

If mDirty_ResetSchedule = True Then
If listCreated = False Then
createList
Else
myRS.MoveFirst
End If

(do some stuff the save changes to reset
schedule that uses the list
End If

If mDirty_PaymentSchedule = True Then
If listCreated = False Then
createList
Else
myRS.MoveFirst
End If

(do some stuff the save changes to Payment
schedule that uses the list)
End If


security_Save_xit:
On Error Resume Next
myQuery.Close
Set myQuery = Nothing
myRS.Close
set myRS = Nothing
Exit Sub

security_Save_err:
MsgBox: Error$,vbCritical, "Trouble In River City"
Resume Security_Save_xit

createList:
Set myQuery = CurrentDB.QueryDefs("qrySecurityList")
With myQuery
.Parameters("theSecurityID") = theSecurityID
Set myRS = .OpenRecordSet(dbOpenDynaSet)
End With
End Sub
----------------------------------------------------------------

The problem is that, regardless of the merits or demerits of this technique,
it doesn't work. That's not valid syntax. There's no way in VB of creating
an inline, internal procedure. I can think of a way to do it, sort of, by
subverting error-handling, but I wouldn't consider that to be good practice
at all.

It seems to me that you are reaching for the concept of the private method
of a class object. You can certainly create a class module to achieve this
sort of isolation; then you can have private methods and data encapsulation
for free. <g> But you'd have to create an instance of the class before
using it.

On the other hand, you could sidestep the overhead of the class object by
creating a separate *standard* module to hold all your Security code. Then
some procedures in that module could be Public and some could be Private.
Then you'd know that the Private ones are only called by other procedures
within that module.

 

[Arvin Meyer [MVP]]

Hi Pete,

I'm not sure what you are saying. The code below uses an outside procedure:

createList

If you wanted to put it in line, you could. Using outside procedures is good
when you either use that procedure for more than a single purpose, or branch
away from a complex procedure most of the time. If you are using it in more
than a single place like perhaps a custom rounding function or an email
function, you need to put it in a standard module. If using it as a branch,
put it in the same form or report module.

But in general, it is preferable to put it in line, within the same sub.
Otherwise it is similar to using a gosub which is now typically considered
archaic programming technique.

 

[PeteCresswell]

The problem is that, regardless of the merits or demerits of this technique,
it doesn't work. That's not valid syntax.

So I found out at compile time when I tried to implement it.

Kind of embarassing to have posted such a dumb question - but I'd have
bet the mortgage money that I'd seen such coding at some time or
another.

Live and learn...

Thanks for the quick response.

 

[Clif McIrvin]

So I found out at compile time when I tried to implement it.

Kind of embarassing to have posted such a dumb question - but I'd have
bet the mortgage money that I'd seen such coding at some time or
another.

Live and learn...

Thanks for the quick response.

Pete, I missed your original post ... gosub works, if you care to use
what Arvin referred to as 'generally considered archaic programming
practice'. <g>

I have a couple procedures (Excel 2003 VBA) that use gosubs for the very
purpose you are describing.

Just code:
gosub createList
instead.