Does this code need improvement?

P

Pwood57

I am developing a DB for volunteer organizations for users with little
or no experience with Access. I have a number of tables and forms
because the DB includes membership, attendance for a number of age
related groups, contributions, ledger, prospects, calendar, event
planning, etc.

To enhance user friendliness I have placed command buttons on most
forms that open other interrelated forms. So I have a lot of code. In
order to reduce the amount of code I created a public sub. I am not
very expereinced in VBA so I would like some input on whether my code
needs improvement.

This code is in the "Global Module":

Public Sub Openmyfrm(stDocName As String)
On Error GoTo Err_Openmyfrm

Dim stLinkCriteria As String

DoCmd.OpenForm stDocName, , , stLinkCriteria

stDocName = Empty
stLinkCriteria = Empty

Exit_Openmyfrm:
Exit Sub
Err_Openmyfrm:
MsgBox Err.Description
Resume Exit_Openmyfrm
End Sub


This is the code for the command buttons on the forms:

Private Sub btnOpenfrmSample_Click()
Openmyfrm "frmSample"
End Sub

Thanks in advance for your input.

Pat Wood
 
T

tina

you don't have to set string variables to "Empty". also, you're declaring a
string variable named stLinkCriteria, but then not assigning any value to
it - so it's useless. suggest the following, as

Public Sub Openmyfrm(ByVal stDocName As String)

On Error GoTo Err_Openmyfrm

DoCmd.OpenForm stDocName

Exit_Openmyfrm:
Exit Sub

Err_Openmyfrm:
MsgBox Err.Description
Resume Exit_Openmyfrm

End Sub

hth
 
M

Marshall Barton

I am developing a DB for volunteer organizations for users with little
or no experience with Access. I have a number of tables and forms
because the DB includes membership, attendance for a number of age
related groups, contributions, ledger, prospects, calendar, event
planning, etc.

To enhance user friendliness I have placed command buttons on most
forms that open other interrelated forms. So I have a lot of code. In
order to reduce the amount of code I created a public sub. I am not
very expereinced in VBA so I would like some input on whether my code
needs improvement.

This code is in the "Global Module":

Public Sub Openmyfrm(stDocName As String)
On Error GoTo Err_Openmyfrm

Dim stLinkCriteria As String

DoCmd.OpenForm stDocName, , , stLinkCriteria

stDocName = Empty
stLinkCriteria = Empty

Exit_Openmyfrm:
Exit Sub
Err_Openmyfrm:
MsgBox Err.Description
Resume Exit_Openmyfrm
End Sub


This is the code for the command buttons on the forms:

Private Sub btnOpenfrmSample_Click()
Openmyfrm "frmSample"
End Sub
Click to expand...


A little more thought might be needed. After you make the
changes that tina suggested, the only benefit the procedure
provides is the error handling. Without your trivial error
handling (probably the same in the calling procedure?), you
have created a one line procedure to simplify one line in
the calling procedure. I.e. the line:
Openmyfrm "frmSample"
and
DoCmd.OpenForm "frmSample"
do the same thing. But when you read your procedure call,
you have to wonder what else the procedure does so whatever
train of thought you were following in the calling procedure
will be distracted while you check the procedure's code.

OTOH, maybe more extensive error handling would provide a
real benefit to using a procedure to open a form.
 
P

Pwood57

HI Marshall,
Thanks for sharing your thoughts. My goal with this code is to
eleminate as many duplicate lines of code as I can from my many Form
Modules. Since my DB addresses so many different needs, I have to use
many tables and forms in order to keep the DB in normal form.

The error handling in my code is what Access generates when I use the
toolbox to create a command button. I do not know enough about error
handling to write my own code. Here is the code I get when I use error
handling from MZTools:

Public Sub Openmyfrm(stDocName As String)
On Error GoTo Openmyfrm_Error

DoCmd.OpenForm stDocName

On Error GoTo 0
Exit Sub

Openmyfrm_Error:

MsgBox "Error " & Err.Number & " (" & Err.Description & ") in
procedure Openmyfrm of Module Global Code"

End Sub

Is this better error handling?
I would appreciate any help you would provide.

Thanks,

Pat Wood
 
P

Pwood57

Thanks, Tina, for your helpful input. The changes you recommended work
just fine.

Thanks,
Pat
 
M

Marshall Barton

Thanks for sharing your thoughts. My goal with this code is to
eleminate as many duplicate lines of code as I can from my many Form
Modules. Since my DB addresses so many different needs, I have to use
many tables and forms in order to keep the DB in normal form.

The error handling in my code is what Access generates when I use the
toolbox to create a command button. I do not know enough about error
handling to write my own code. Here is the code I get when I use error
handling from MZTools:

Public Sub Openmyfrm(stDocName As String)
On Error GoTo Openmyfrm_Error

DoCmd.OpenForm stDocName

On Error GoTo 0
Exit Sub

Openmyfrm_Error:

MsgBox "Error " & Err.Number & " (" & Err.Description & ") in
procedure Openmyfrm of Module Global Code"

End Sub

Is this better error handling?
I would appreciate any help you would provide.

Pat Wood
Click to expand...


I wasn't trying to suggest that you need more/better error
handling, just that without some more significant
dunctionality, your procedure doesn't appear to provide any
benefit. Maybe you have plans to add additional features
later that will justify having this procedure. My only
point was that there is a bit of a downside to adding
procedures just for the sake of having more procedures.
 
T

tina

you're welcome :)

Marshall has a very good point, though, Pat. you're really replacing a
one-line procedure with a one-line procedure that calls another one-line
procedure. i usually don't use error handling code in such a simple
procedure; the exception would be when the action (OpenForm in this case)
might fail for various reasons.

to actually reduce the code in your module(s) a bit, you might turn the
Public Sub into a Public Function (just replace the word Sub with the word
Function, everywhere it appears in the procedure). the advantage of a
function is that it can be called directly from the Click event line in the
command button's Properties box, as

=Openmyfrm("frmSample")

so you don't have to create an OnClick event procedure for each button.

hth
 
P

Pwood57

Thanks Tina, that was just what I needed and it works great. I don't
know why I have not seen this before in my reading books on Access and
on the web. Thank you for the tip.

Pat
 
T

tina

you're welcome :)


Thanks Tina, that was just what I needed and it works great. I don't
know why I have not seen this before in my reading books on Access and
on the web. Thank you for the tip.

Pat
Click to expand...
 
Top