Variable Not Being Set in For Each Properly

[Andrew Johnson]

Well I am almost done finalizing the auditing portion of an application. The way I am doing it is looping through all text fields, drop down boxes and checkboxes and storing their values in the form_load event. Then I am doing the same thing in the form_afterUpdate event and comparing the two. If there is a difference I am logging it, if not I move on. Here is the code:

    Dim strValues(1 To 32) As String

    Private Sub Form_AfterUpdate()
        Dim strCurrentValue, strSQL As String
        Dim intCurrentField As Integer
        intCurrentField = 1
    
        For Each C In Forms!frmVendorsManageVendors.Controls
            Select Case C.ControlType
                Case acTextBox, acComboBox, acCheckBox
                    'Doing this because I don't want a NULL as it won't concatenate in the SQL query and don't want 0 or -1 for the boolean fields
                    strCurrentValue = IIf(IsNull(C), "", IIf(C = vbTrue Or C = vbFalse, IIf(C = vbTrue, "Yes", "No"), C))
                
                    If strValues(intCurrentField) <> strCurrentValue Then
                        strSQL = "INSERT INTO changesTable (change_time,user_affected,field_affected,old_value,new_value) VALUES (NOW()," & [id] & ",'" & C.ControlSource & "','" & strValues(intCurrentField) & "','" & strCurrentValue & "')"
                    
                        DoCmd.SetWarnings False
                        DoCmd.RunSQL strSQL
                        'InputBox "", "", strSQL
                        strSQL = "WEEEE"
                        DoCmd.SetWarnings True

                        strValues(intCurrentField) = strCurrentValue
                    End If
                
                    intCurrentField = intCurrentField + 1
            End Select
        Next
    End Sub

    Private Sub Form_Open(Cancel As Integer)
        Call btnLock_Click
    
        Dim intCurrentField As Integer
        intCurrentField = 1
    
        For Each C In Forms!frmVendorsManageVendors.Controls
            Select Case C.ControlType
                Case acTextBox, acComboBox, acCheckBox
                    'Doing this because I don't want a NULL as it won't concatenate in the SQL query and don't want 0 or -1 for the boolean fields
                    strValues(intCurrentField) = IIf(IsNull(C), "", IIf(C = vbTrue Or C = vbFalse, IIf(C = vbTrue, "Yes", "No"), C))
                    intCurrentField = intCurrentField + 1
            End Select
        Next
    End Sub

As you can see there is a commented out line where I insert into the changesTable that will put up the query in an input box so I can copy/paste it and look at it. When I uncomment that line everything is fine. If it is commented it generates the first change fine, but then won't change it for the other controls. So if I change field1 and field2 it will insert the field 1 change twice.

It is quite confusing and I have NO CLUE as to why this is happening.

 

[Uncle Gizmo]

I believe what you are trying to do has been done many times before. So it may pay you to take a bit of time studying other people's solutions.

I would start off by searching this forum for "Audit Trail"

there is a Microsoft web page on it HERE

and I think you may get better results if you use the Microsoft type of logic which is based on the "old value" see this line here:
If IsNull(C.OldValue) Or C.OldValue = "" Then

In my opinion Microsoft's attempt is a bit flawed, (I'm referring to the one where they store changes in a memo field) I think it's far better to store the changes in the table, however I haven't seen a really good implementation of this idea yet.

I have progressed my own ideas quite a long way, but I haven't had time to complete them. It's about time this was revisited, and I will be following this thread with interest.

 

[Andrew Johnson]

I believe what you are trying to do has been done many times before. So it may pay you to take a bit of time studying other people's solutions.

I would start off by searching this forum for "Audit Trail"

there is a Microsoft web page on it HERE

and I think you may get better results if you use the Microsoft type of logic which is based on the "old value" see this line here:
If IsNull(C.OldValue) Or C.OldValue = "" Then

In my opinion Microsoft's attempt is a bit flawed, (I'm referring to the one where they store changes in a memo field) I think it's far better to store the changes in the table, however I haven't seen a really good implementation of this idea yet.

I have progressed my own ideas quite a long way, but I haven't had time to complete them. It's about time this was revisited, and I will be following this thread with interest.

Hi, I've actually come up with a table that covers all things about the audit needed and have produced the code to do so (I actually used a lot of code from that MS Support link you provided) -- my problem is some kind of weird abnormality... I posted this question on stack overflow (can't post links yet but search: "For each not working properly in VBA code") and a user there suggested looking into the fact that the inputbox is taking focus away from the form I cycling through which sounds like a promising lead...

 

[stopher]

I tried your code and was unable to duplicate your error. It worked fine within one record i.e. I could change several controls and those changes where recorded to the changes table correctly.

However, there seems to be a bigger flaw at the moment. You are using the On Open event to store the first values. This only stores the values for the first record. And while you are re-setting the stored values for the current record as you go, when you change to a new record, you have not re-stored the values for the next record i.e. it is still holding the values from the last record.

One would recommend the BeforeUpdate event to grab the old values. This approach is used in Allen Browne's method here.

BTW, you can use Debug.print strSQL to show your string in the "immediate" window (make sure the immediate window is visible)

Chris

 

[stopher]

Adding to my point about the second problem, here's a variation of your code and taking Uncle Gizmo's point about OldValue into account:

Private Sub Form_BeforeUpdate(Cancel As Integer)
    
        Dim intCurrentField As Integer
        Dim OldValue As Variant
        intCurrentField = 1

        For Each c In Forms!frmVendorsManageVendors.Controls
            Select Case c.ControlType
                Case acTextBox, acComboBox, acCheckBox
                    OldValue = c.OldValue
                    'Doing this because I don't want a NULL as it won't concatenate in the SQL query and don't want 0 or -1 for the boolean fields
                    strValues(intCurrentField) = IIf(IsNull(OldValue), "", IIf(OldValue = vbTrue Or OldValue = vbFalse, IIf(OldValue = vbTrue, "Yes", "No"), OldValue))
                    
                    intCurrentField = intCurrentField + 1
            End Select
        Next
End Sub

Note that this code is now in the BeforeUpdate event and grabs the correct values using the OldValue property. Note that you should remove the On Open code as it will be redundent.

Personally I'd write the code a little differently for clarity but I've left it as is so you are familiar with it.

hth
Chris

 

[Andrew Johnson]

An excellent point about multi-records. I've been testing with only one but you are 100% right. I've fixed this by moving my form_load code to form_current.

The problem with the same query executing persists however. I've added that debug.print line to my code but can't seem to figure out where it's printing to... I've never used that function before, any tips would be welcomed.

 

[stopher]

In the VBA editor, type Ctrl+G (or click View->Immediate Window from the menu). This will show a blank window where debug.print will output to. This window can also be use directly for debugging.

Perhaps you could post your database.

Chris

 

[Andrew Johnson]

I can't post my database because of ownership issues.

However I can post my updated code:

Private Sub Form_BeforeUpdate(Cancel As Integer)
    Dim C As Control
    For Each C In Controls
        Select Case C.ControlType
            Case acTextBox, acComboBox, acCheckBox
                Dim strOriginalValue, strCurrentValue, strSQL As String

                strOriginalValue = IIf(IsNull(C.OldValue), "", IIf(C.OldValue = vbTrue Or C.OldValue = vbFalse, IIf(C.OldValue = vbTrue, "Yes", "No"), C.OldValue))
                strCurrentValue = IIf(IsNull(C.Value), "", IIf(C.Value = vbTrue Or C.Value = vbFalse, IIf(C.Value = vbTrue, "Yes", "No"), C.Value))

                If strOriginalValue <> strCurrentValue Then
                    strSQL = "INSERT INTO fringefestival_changes (change_time,change_admin,action_taken,user_affected,year_affected,field_affected,type_affected,old_value,new_value) VALUES (NOW(),'" & ThisUserName() & "','Edit'," & [id] & ",0,'" & C.ControlSource & "','Administrator','" & Replace(strOriginalValue, "'", "") & "','" & Replace(strCurrentValue, "'", "") & "')"
                    Debug.Print "Before: " & strSQL
                    CurrentDb.Execute strSQL
                    Debug.Print "After: " & strSQL
                End If
        End Select
    Next
End Sub

Code is simpler and more efficient but the problem persists. The really weird part is the strings printed to the debugger are displaying CORRECTLY. I have taken screenshots but can't post URLs directly.

The link to the screenshots of the debugger are:

• www . andrew-g-johnson . com / access1 . jpg
• www . andrew-g-johnson . com / access2 . jpg

The link to the screenshot of the results is:

• www . andrew-g-johnson . com / access3 . jpg

I have no idea why this would be happening and am ready to pull my hair out.

Sorry for the "link spam" but I think it needs to be shown.

 

[stopher]

Your latest code seems to work fine with me.

What type of controls are you updating? Are you just using one form or do you have subforms?

BTW, I see you are storing the changes in the before update event. You run the potential of recording a change that for some reason doesn't actually happen.

Chris