Is this a bad practice? (1 Viewer)

[GLese]

I'm new to Access/VBA and I want to get opinions from y'all who have more experience on whether or not this is a bad practice while I'm early on in my database design/skill development.

My database has a bunch of forms which allow FE users to enter and selectively edit information. I am including on all FE forms an "Exit" button which runs a simple sub that closes the open form without saving and returns the user to what will be their home screen (a nav form with buttons for various functions).

Is it bad practice if all of the exit buttons have the same name if they all run the same function? (I've been using btnExit for simplicity sake)

Or should I be changing the name to reflect what form it's on? (ex: btnExitPwLogin)

Look forward to y'alls opinions so I can become educated and make the best design decisions going forward!

 

[Ranman256]

no need for more: Docmd.close
is all you need.
this will cause no errors.

 

[isladogs]

My database has a bunch of forms which allow FE users to enter and selectively edit information. I am including on all FE forms an "Exit" button which runs a simple sub that closes the open form without saving and returns the user to what will be their home screen (a nav form with buttons for various functions).

Is it bad practice if all of the exit buttons have the same name if they all run the same function? (I've been using btnExit for simplicity sake)

Or should I be changing the name to reflect what form it's on? (ex: btnExitPwLogin)

As long as the code is a Private Sub, its fine to use the same name for each

However if the form is bound (which is the normal method), changes are saved automatically as soon as you move to another control or record.
So the data will be saved when you close the form UNLESS you EXPLICITLY prevent that happening

If you are using an UNBOUND form, its different BUT those are much more effort to work with

 

[GLese]

As long as the code is a Private Sub, its fine to use the same name for each

They are Private subs on each form. Thanks!

However if the form is bound (which is the normal method), changes are saved automatically as soon as you move to another control or record.
So the data will be saved when you close the form UNLESS you EXPLICITLY prevent that happening

If you are using an UNBOUND form, its different BUT those are much more effort to work with

I'm pretty sure my forms are bound, they pull a record source from a specific table if you are editing, or add to that table if it's a new entry. as far as I have seen, the way I have it structured it isn't changing any data in the tables unless I am instructing it to. (I don't see any partial records added, and if I'm editing them, it doesn't change the timestamp fields I have added unless directed to do so). Because we don't want our FE users to have a lot of access I'm keeping what they can and can't edit pretty locked down.

 

[isladogs]

They are Private subs on each form. Thanks!

I'm pretty sure my forms are bound, they pull a record source from a specific table if you are editing, or add to that table if it's a new entry. as far as I have seen, the way I have it structured it isn't changing any data in the tables unless I am instructing it to. (I don't see any partial records added, and if I'm editing them, it doesn't change the timestamp fields I have added unless directed to do so). Because we don't want our FE users to have a lot of access I'm keeping what they can and can't edit pretty locked down.

Sounds like the forms are bound. In design view, check whether the controls have field names (bound) or if these say unbound.

Also sounds like you've sorted it to only save when instructed to do so

 

[GLese]

Sounds like the forms are bound. In design view, check whether the controls have field names (bound) or if these say unbound.

Also sounds like you've sorted it to only save when instructed to do so

Yarp, they are bound forms. Thanks for the direction!

 

[Pat Hartman]

GLese,
The ONLY event that you can reliably use to save or not save a record is the form's BeforeUpdate event. Think of that event as the flap on a funnel. If you decide the data is good, you open the flap and allow the data to be saved. If you decide the data should not be saved - for whatever reason, you keep the flap closed by cancelling the BeforeUpdate event. If you also want to undo any pending changes, you would use Me.Undo.

Personally, I would use Me.Undo in only TWO situations.
1. The user is not allowed to edit the record at all. That means I'm not going to give him the ability to change his data to correct it. I am simply NOT going to allow it to be saved - period. So Me.Undo followed by Cancel = True and Exit Sub solves all problems. Many people go to extremes to lock/unlock etc. That gets very tricky and can prevent buttons and search combos from working when most of the time, you can simply catch and prevent by using the Form's BeforeUpdate event.
2. If I require confirmation and so ask the user if he wants to save and if he says NO, I use Me.Undo and Cancel = True followed by Exit Sub to get rid of the dirty data.

Any time you use other events to validate data or attempt to control whether or not data will be saved, you WILL save bad data. No one ever manages to close all the gaps. Access is very persistent and takes it as its most important mission to save your data. You can only truly control this by properly using the Form's BeforeUpdate event. Using the BeforeUpdate event of a control, can be used for some validation but unless, the code needs to hide/show or lock/unlock other controls, I generally just put the validation in the Form's BeforeUpdate event. That makes it easier for me to see all validation in one place and not have to view dozens of events looking for issues.

The Access event model is not arbitrary. It is designed to allow you to control actions based on what triggered them. Occasionally, code will work just as well in one place as another but that does not apply to validation and the decision to save or not to save. Use the Form's BeforeUpdate event - The buck stops there.

To answer your actual question - just look to what Access does. When it creates event procedures on forms, they are all named identically. So, we should do the same thing for consistency. All my Save, Close, Edit, Next, etc buttons have identical names. That way, they can have identical code. Some of them use common procedures built in standard modules so they are accessible by all forms.

 

[The_Doc_Man]

Pat said it but I'll approach this another way.

By product specification, the code in a form or report's Class Module is ALWAYS Private even if you TRIED to declare it Public. Therefore, there is never any confusion about the name of a control - because without extra effort, THAT is not "in scope" either. Besides which, if a form isn't open, its contents are not instantiated (existing in memory) and thus the potentially offending control overlap can't occur - because one of the participants in the overlap doesn't exist at the moment.

Therefore, don't worry about name overlaps among similar functions on different forms. NOTE, however, that General Modules CAN contain things declared as Public, and there, you COULD have name overlap.

 

[GLese]

GLese,
The ONLY event that you can reliably use to save or not save a record is the form's BeforeUpdate event. Think of that event as the flap on a funnel. If you decide the data is good, you open the flap and allow the data to be saved. If you decide the data should not be saved - for whatever reason, you keep the flap closed by cancelling the BeforeUpdate event. If you also want to undo any pending changes, you would use Me.Undo.

Personally, I would use Me.Undo in only TWO situations.
1. The user is not allowed to edit the record at all. That means I'm not going to give him the ability to change his data to correct it. I am simply NOT going to allow it to be saved - period. So Me.Undo followed by Cancel = True and Exit Sub solves all problems. Many people go to extremes to lock/unlock etc. That gets very tricky and can prevent buttons and search combos from working when most of the time, you can simply catch and prevent by using the Form's BeforeUpdate event.
2. If I require confirmation and so ask the user if he wants to save and if he says NO, I use Me.Undo and Cancel = True followed by Exit Sub to get rid of the dirty data.

Any time you use other events to validate data or attempt to control whether or not data will be saved, you WILL save bad data. No one ever manages to close all the gaps. Access is very persistent and takes it as its most important mission to save your data. You can only truly control this by properly using the Form's BeforeUpdate event. Using the BeforeUpdate event of a control, can be used for some validation but unless, the code needs to hide/show or lock/unlock other controls, I generally just put the validation in the Form's BeforeUpdate event. That makes it easier for me to see all validation in one place and not have to view dozens of events looking for issues.

The Access event model is not arbitrary. It is designed to allow you to control actions based on what triggered them. Occasionally, code will work just as well in one place as another but that does not apply to validation and the decision to save or not to save. Use the Form's BeforeUpdate event - The buck stops there.

To answer your actual question - just look to what Access does. When it creates event procedures on forms, they are all named identically. So, we should do the same thing for consistency. All my Save, Close, Edit, Next, etc buttons have identical names. That way, they can have identical code. Some of them use common procedures built in standard modules so they are accessible by all forms.

Pat thanks for your answer here, I'm dropping the code I have written for the login button I have set up. This is almost identical code for several other places but the Me.DateIn and Time in bits get changed to "Picked" and "Out" fields and the MsgBox at the end gets some different wording based on the form.

Private Sub btnLoginPaperwork_Click()
Dim ctl As Control
Dim CName As String
For Each ctl In Me.Controls
  If ctl.Tag = "Marked" Then
     If Nz(ctl, "") = "" Then
       CName = ctl.Controls(0).Caption
MsgBox "Critical Information Missing.  All red shaded boxes must be filled in!", vbCritical + vbOKOnly, "Error!"
       ctl.SetFocus
       Exit Sub
     End If
   End If
Next ctl
    Me.DateIn = Date
    Me.TimeIn = Time()
    'Timestamps DateIn mm/dd/yyyy, and TimeIn column hhmm (24hr)
    MsgBox "Paperwork Sucessfully Logged In", vbOKOnly, "Thank You!"
    CloseWind
End Sub

If I am understanding what you say in your post, I should move the first part of all this to the before update section of my form? (the bit that checks for "Marked" tags and returns the first msgbox)

I tried putting that bit of code in a standard module as a public sub earlier, and managed to get the Me.Controls to work with some code change. But what I discovered is that if I was missing information in the Marked tagged controls, then the error box would pop up, I'd click okay, and then the rest of the code would run on the sub in the form itself, timestamp my designated fields and save the record. Which is of course, NOT what I want to happen.


Would I be better off putting the whole block of code (since it will be the same across the forms I need it for) into a standard module or class module (not sure which would be best, I haven't quite grasped when a class module is more appropriate) and setting up my code in a way that I can define in the form's sub what fields should be timestamped/messagebox should display using a series of Dim statements in the form sub?

((I really appreciate y'alls patience and kind help, I think I'm starting to get the hang of the lingo and general concepts of programming))

 

[Pat Hartman]

If you want to make this code generic, you can do it by passing in a form reference.

Private Sub btnLoginPaperwork_Click()
    Call LoginPaperwork(Me)
End Sub

Then you have to change the sub to reference the form object passed to it. This code goes into a standard module - NOT a form or class module.

Public Sub LoginPaperwork(frm as Form)
Dim ctl As Control
Dim CName As String
For Each ctl In frm.Controls
  If ctl.Tag = "Marked" Then
     If Nz(ctl, "") = "" Then
       CName = ctl.Controls(0).Caption
MsgBox "Critical Information Missing.  All red shaded boxes must be filled in!", vbCritical + vbOKOnly, "Error!"
       ctl.SetFocus
       Exit Sub
     End If
   End If
Next ctl
    frm.DateIn = Date
    frm.TimeIn = Time()
    'Timestamps DateIn mm/dd/yyyy, and TimeIn column hhmm (24hr)
    MsgBox "Paperwork Sucessfully Logged In", vbOKOnly, "Thank You!"
    CloseWind
End Sub

However, since this is validation code, it should be called from the form's BeforeUpdate event rather than a button click event. Just move the Call code to the BeforeUpdate event.

 

[GLese]

If you want to make this code generic, you can do it by passing in a form reference.

Ok that makes sense. Could I set it up even more generic so that where you changed it to

frm.DateIn = Date
frm.TimeIn = Time

I could replace the DateIn with something like strDate

then in the part on the form have a statement

Dim strDate as String
strDate = DateIn

and just change it from DateIn to the other fields I will use on other forms (DatePicked and DateOut) with a similar setup for the associated time fields? Or does that break some coding laws I'm unaware of?

However, since this is validation code, it should be called from the form's BeforeUpdate event rather than a button click event. Just move the Call code to the BeforeUpdate event.

If I move all this to the BeforeUpdate, since I want the FE Users to click a button to save the record/edits and the button click kick them to whatever the next form in the sequence is, do I code the button click to just close the form and the BeforeUpdate even will run the rest? Or do I tell the button click to run the before update event?

 

[Pat Hartman]

I could replace the DateIn with something like strDate

Not easily. A Function returns one and only one value and you now need three.

When I write generalized code like this, I just make the controls on the forms all have the same Name property. It doesn't matter what the actual table column name is. I can control it with the control's Name property.

If I move all this to the BeforeUpdate, since I want the FE Users to click a button to save the record/edits and the button click kick them to whatever the next form in the sequence is, do I code the button click to just close the form and the BeforeUpdate even will run the rest? Or do I tell the button click to run the before update event?

Feels like I've answered this question at least three times this week.

Although I sometimes put a save button on a form because it gives the users some positive action, I don't generally make the save dependent on it. If Access takes it upon itself to save the record because the user scrolled or clicked into the subform, I just let it go. The less I try to get in the way of Access, the less likely I am to create a bug caused because I've missed some instance where I didn't know Access would save. You can have some control by using a form level variable. In the header of your form's class module:

Public bSaveOK as Boolean

Then in various events, you set and check this value.
Current event - bSaveOK = False
Click event of your save button - bSaveOK = True, then save the current record and let the BeforeUpdate event take over
BeforeUpdate event of Form -
If bSaveOK = True Then
Else
Msgbox "Please press the Save button to save.",vbOKOnly
Cancel = True
Me.cmdSave.SetFocus
Exit Sub
End If
AfterUpdate event of Form - bSaveOK = False

You will probably also need code in the Unload Event to prevent a form from being closed if the save was unsuccessful. Add an error handler to catch 2501 which is the error that will get raised of the save event is cancelled. when this happens, you want to cancel unloading the form.

Select Case Err.Number
Case 2501
Cancel = True
Case Else
Msgbox Err.Number & "--" & Err.Description
End Select

 

[GLese]

Although I sometimes put a save button on a form because it gives the users some positive action, I don't generally make the save dependent on it. If Access takes it upon itself to save the record because the user scrolled or clicked into the subform, I just let it go. The less I try to get in the way of Access, the less likely I am to create a bug caused because I've missed some instance where I didn't know Access would save. You can have some control by using a form level variable.

After letting my brain relax this weekend I have a better grasp I think of what you mean by not getting in the way of Access. So I did some digging around and found what seems to be a more involved data validation code which I have dropped into a module and am calling in the BeforeUpdate part of my form. It is giving the right error message boxes based on what is missing (Took your advice and made my controls have generic names across the forms this will be used on).

I've coded in a few things to the "save buttons" and maybe this is overly redundant, but it seems to work. I'd appreciate a lookover from y'all who have more experience in all this...

In a Module: modUtilities

Public Function RequiredData(ByVal TheForm As Form) As Boolean
'Check that all controls with the tag "Marked" have data entered
    Dim Ctl As Control
    On Error GoTo Err_RequiredData
RequiredData = False
For Each Ctl In TheForm
    If Ctl.Tag = "Marked" Then
        If Ctl = "" Or IsNull(Ctl) Then
        RequiredData = True
        Exit For
        End If
    End If
Next Ctl
If RequiredData = True Then
    MsgBox "You must fill out " & Ctl.Controls(0).Caption & "," & vbCr & _
        "Please make sure this information is accurate.", _
        vbInformation, "Required Information!"
Else
    RequiredData = False
End If
Exit_RequiredData:
    On Error Resume Next
        If Not (Ctl Is Nothing) Then
            Set Ctl = Nothing
        End If
    Exit Function
Err_RequiredData:
    Select Case Err
        Case 0
            Resume Next
        Case Else
            MsgBox "Error: " & Err.Number & vbCrLf & vbCrLf & Err.Description, _
            vbInformation
    End Select
End Function

And here's what's on the form itself:
btnPhySmpSE (Save and Exit Button)
btnPhySmpSLA (Save and Login Another button)

Option Compare Database
Option Explicit
Private Sub btnExit_Click()
CloseWind
End Sub
Private Sub btnPhySmpSE_Click()
If RequiredData(Me) = True Then
    'Do Nothing
    Else
    Me.DateStamp = Date
    Me.TimeStamp = Time()
    DoCmd.Save
    MsgBox "Sample Logged In!", vbOKOnly, "Thank you!"
    CloseWind
End If
End Sub
Private Sub btnPhySmpSLA_Click()
    If RequiredData(Me) = True Then
    'Do Nothing
    Else
    Me.DateStamp = Date
    Me.TimeStamp = Time()
    DoCmd.Save
    MsgBox "Sample Logged In!", vbOKOnly, "Thank you!"
    DoCmd.Close
    DoCmd.OpenForm "frmPhySmpLogin", acNormal
    End If
End Sub
Private Sub Form_BeforeUpdate(Cancel As Integer)
If RequiredData(Me) Then
    Cancel = True
    Else
    'Do Nothing
End If
End Sub

 

[Pat Hartman]

Saving date and time separately only makes the data harder to work with. Just use a field named something like LastChangedDT and set it to Now() when the record is saved. But set it in the form's BeforeUpdate event NOT in a button click. You don't want to dirty the record yourself. You only want to populate this field if someone else has already dirtied the record and Access has started the process to save it.

 

[GLese]

Saving date and time separately only makes the data harder to work with. Just use a field named something like LastChangedDT and set it to Now() when the record is saved. But set it in the form's BeforeUpdate event NOT in a button click. You don't want to dirty the record yourself. You only want to populate this field if someone else has already dirtied the record and Access has started the process to save it.

Okay, I'll look into this. Thanks Pat!

Also I'm going to mark this as closed, since I feel my questions have been answered. Thanks to everyone for their help!

 

[Pat Hartman]

You're welcome.