Code Clarity

  • Thread starter Thread starter John
  • Start date Start date
J

John

I recently asked a question about calling a function,
which was answered, but in turn was told my code
was "convoluted." Perhaps the reader of the code did not
understand it's purpose, but I was wondering if you could
take a look and tell me if it looks good.

The code is in an afterupdate event on an unbound text
box. It's purpose is to either
a) move the form to the record specified in the text box
b) open a separate search form if an "s" is entered
c) open a custom error dialog if the client does not exist
d) display a message box if the search could not run

Here it is, with the new bit:


Private Sub txtClient_AfterUpdate()

On Error GoTo err_clientUpdate '<-- No records or
'invalid use of null

If txtClient = "s" Then
'The OpenSearchForm function opens and binds an
'unbound form for searching for a specific record.
Call OpenSearchForm("Client")
Exit Sub
End If

'I use the following to search

DoCmd.GoToControl "txtClientID" '<--- Bound Field
DoCmd.FindRecord [txtClient]
DoCmd.GoToControl "txtClientID"

If txtClientID <> txtClient Then '<--- If search fails.

'dlgError is an always-open form which is used
'for error display. Note: Not actual errors,
'but errors as the user percieves them.

Forms!dlgerror.Caption = "Invalid Client Number"
Forms!dlgerror!txtTitle = "Invalid Client Number"
Forms!dlgerror!txtError = "The client number you
_entered does not exist. If you wish to add a
_new client, simply click the 'New'button. You
_can either click the 'Search' button or type
_an 's' into the clientID field to search for a
_specific client."
Forms!dlgerror.Visible = True
End If

exit_ClientUpdate:
Exit Sub

err_clientUpdate:
MsgBox "You have no clients currently entered into
_the system, or you did not enter anything
_into the Go To field.", , "Error!"
Exit Sub

End Sub
 
Ouch! that must have hurt. Sorry.
Have you considered using a combobox for this?

Regards
Habib
 
It looks ok to me. The only thing I might change is to
trap the null value right off the top.

If Not Isnull(txtClient) Then
If txtClient = "s"

...
...
End If
End If

You could probably make the code where you're setting some
properties on the dlgerror form more modular. You could
put it in it's own subroutine and pass the form in as an
object. For example: Sub showError(frm as Object) --- to
call this you would say ... showError(Forms!dlgerror).
This way, your code is more reusable and you'll probably
find that, in the long run, you'll code less because you
already have these routines and functions in place, you
just need to call them.

Other than that, it looks fine to me.

Regards,
Jen
 
The problem with a combo box is that this table could
ultimately contain thousands of records, and combo boxes
tend to get slow and cluttered when that happens.
 
Exactly...
-----Original Message-----
The problem with a combo box is that this table could
ultimately contain thousands of records, and combo boxes
tend to get slow and cluttered when that happens.


.
Click to expand...
 
Back
Top