# VBA save multiple records code, 2 for loops or change to 1 for loop with if statement inside

I am becoming more critical of the code I write. This is leading me to questions like "Could I write my code to be more efficient?" Of course the answer to that is a resounding YES. So, I posit the question to those who know much more than I do. What could I do to make this more efficient?

The purpose of this code is to take data from an Access form and save it into two (or three if a condition is met) tables.

Note: I originally asked a form of this question on Stack Overflow.

Private Function SaveSerials()
Dim rs As DAO.Recordset
Dim rs2 As DAO.Recordset
Dim rs3 As DAO.Recordset
Dim qry As QueryDef
Dim i as Long
If Not BasicInclude.DebugMode Then On Error GoTo Error_Handler Else On Error GoTo 0
Dim out As String: out = Validate(Me.Name, msclblstyle)
If out = "" Then
Set qry = dbLocal.QueryDefs("qryDeletefromLabelTemp")
qry.Parameters(0) = "%1" & Me.txtOperator.Value & "%"
qry.Execute
Set rs = dbLocal.TableDefs("dbo_Part_Serial_Number").OpenRecordset(dbOpenDynaset, dbSeeChanges)
Set rs2 = dbLocal.TableDefs("dbo_Label_Trace_Temp").OpenRecordset(dbOpenDynaset, dbSeeChanges)
For i = 1 To Me.lstData.ListCount - 1
rs.Fields("Part_No").Value = Me.txtPart_No.Value
rs2.Fields("PartNumber").Value = Me.txtPart_No.Value
rs.Fields("Label_Date").Value = Me.txtLabel_Date.Value
rs.Fields("Serial_No").Value = Me.lstData.Column(0,i)
rs2.Fields("PartSerialNumber").Value = Me.lstData.Column(0,i)
rs.Fields("Operator").Value = Me.txtOperator.Value
rs2.Fields("Operator").Value = "%1" & Me.txtOperator.Value & "%"
rs.Fields("Work_Order").Value = Me.cboWork_Order.Value
rs2.Fields("WorkOrder").Value = Me.cboWork_Order.Value
rs.Fields("Date_Code").Value = Me.txtDate_Code.Value
rs2.Fields("Date_Code").Value = Me.txtDate_Code.Value
rs.Fields("Notes").Value = Me.txtNotes.Value
rs.Fields("Flex_Circuit_Date_Code").Value = Me.txtFC_Date_Code.Value
rs.Fields("Flex_Circuit_Int_Date_Code").Value = Me.txtFCI_Date_Code.Value
rs.Fields("Substrate_Date_Code").Value = Me.txtSubstrate_Date_Code.Value
rs.Fields("SMF_Date_Code").Value = Me.txtSMF_Date_Code.Value
rs.Fields("r").Value = Me.chkR.Value
rs.Update
rs2.Update
Next
If Me.txtPart_No Like "PS*" Then
Set qry = dbLocal.QueryDefs("qryClear_tbl_BarcodeList")
qry.Parameters(0) = Environ(GetEnvironString(esComputerName))
qry.Execute
Set rs3 = dbLocal.TableDefs("tbl_BarcodeList").OpenRecordset(dbOpenDynaset, dbSeeChanges)
For i = 1 To Me.lstData.ListCount - 1
rs3.Fields("Part_No").Value = Me.txtPart_No.Value
rs3.Fields("Host").Value = Environ(GetEnvironString(esComputerName))
rs3.Fields("Serial_No").Value = Me.lstData.Column(0,i)
rs3.Update
Next
DoCmd.OpenReport "rpt_BarcodeList", acViewPreview
End If
SaveSerials = 0
Else
MsgBox "Please fill out the following fields: " & vbCrLf & out, vbCritical, "Error"
SaveSerials = 1
End If
Error_Exit:
set rs = Nothing
set rs2 = Nothing
set rs3 = Nothing
Exit Function
Error_Handler:
MsgBox "The following error has occured" & vbCrLf & vbCrLf & _
"Error Number: " & Err.Number & vbCrLf & _
"Error Source: SaveSerials" & vbCrLf & _
"Error Description: " & Err.Description _
, vbOKOnly + vbCritical, "An Error has Occured!"
SaveSerials = -1
Resume Error_Exit
End Function

• Welcome to CR! Please describe the purpose of your code (the post title should roughly summarize that), give as much context to reviewers as possible - see how to get the best value out of Code Review for all the info, or Simon's guide for posting a good question for a quick read. – Mathieu Guindon Nov 9 '17 at 20:54
• @Mat'sMug in line 7 does Else On Error GoTo 0 have any effect? – user109261 Nov 10 '17 at 16:00
• @ThomasInzina seems so.. Arguably looks like a case for precompiler directives though. – Mathieu Guindon Nov 10 '17 at 16:03
• @Mat'sMug I was thinking the same thing. I recently seem that pattern#Const DebugMode in the public module with #If DebugMode .... #Else ... – user109261 Nov 10 '17 at 16:07
• the debugmode variable is set when the switchboard loads based on the user account. I also sometimes set that to true via the immediate window when i am not on my own computer so i can see where the fail happens. – KySoto Nov 10 '17 at 16:10

On Error GoTo 0 is used in conjunction with On Error Resume Next to reset any errors that may occur between the two statements.

For this reason, On Error GoTo 0 in the ↓ line below ↓ should be changed to On Error Resume Next.

If Not BasicInclude.DebugMode Then On Error GoTo Error_Handler Else On Error GoTo 0


I would rewrite the Validation statement removing the else clause. This will make the code easier to read.

If out <> "" Then
MsgBox "Please fill out the following fields: " & vbCrLf & out, vbCritical, "Error"
SaveSerials = 1
Goto Error_Exit:
End If


Setting objects to nothing is rarely necessary with today's garbage collectors.

set rs = Nothing
set rs2 = Nothing
set rs3 = Nothing


Recordset should be closed.

rs.Close
rs2.Close
rs3.Close


Grouping your recordset field will improve the readability of the code, make it easier to spot missing fields, and just makes sense.

For i = 1 To Me.lstData.ListCount - 1
'Add record to dbo_Part_Serial_Number
rs.Fields("Part_No").Value = Me.txtPart_No.Value
rs.Fields("Label_Date").Value = Me.txtDate_Created.Value
rs.Fields("Serial_No").Value = Me.lstData.Column(i)
rs.Fields("Operator").Value = Me.txtOperator.Value
rs.Fields("Work_Order").Value = Me.cboWork_Order.Value
rs.Fields("Date_Code").Value = Me.txtDate_Code.Value
rs.Fields("Notes").Value = Me.txtNotes.Value
rs.Fields("Flex_Circuit_Date_Code").Value = Me.txtFC_Date_Code.Value
rs.Fields("Flex_Circuit_Int_Date_Code").Value = Me.txtFCI_Date_Code.Value
rs.Fields("Substrate_Date_Code").Value = Me.txtSubstrate_Date_Code.Value
rs.Fields("SMF_Date_Code").Value = Me.txtSMF_Date_Code.Value
rs.Fields("r").Value Me.chkR.Value
rs.Update

'Add record to dbo_Label_Trace_Temp
rs2.Fields("PartNumber").Value = Me.txtPart_No.Value
rs2.Fields("PartSerialNumber").Value = Me.lstData.Column(i)
rs2.Fields("Operator").Value = "%1" & Me.txtOperator.Value & "%"
rs2.Fields("WorkOrder").Value = Me.cboWork_Order.Value
rs2.Fields("Date_Code").Value = Me.txtDate_Code.Value
rs2.Update
Next


You may consider extracting code related to the three recordset into their own subroutines but it is not necessary.

It is hard to discern the meaning of the return value of SaveSerials (e.g. -1, 0, 1) without studying the code. Consider using an Enum.

• I use the on error goto 0 for troubleshooting and development. For production I want the user to just see the error message and let me know. The output is only being used to determine if the form is going to be closed or not. i didnt really need to set the error path to -1, its just habit. – KySoto Nov 10 '17 at 15:55
• In that case Function SaveSerials() As Boolean makes more sense. – user109261 Nov 10 '17 at 15:59