Is there a better way to write this Access SQL expression?

Question

I have an Access database that I use to track personnel going on trips. When I add someone to a trip, it copies over some information items from the master personnel table into another table that ties that person to the trip and then displays readiness things that I need to track. I'm in the process of updating how the front end talks to the back end in preparation for migrating this over to a proper SQL server rather than just a backend file on a share drive, and was wondering if there was a better way to code this.

Here's the original code:

        Dim rst As DAO.Recordset
        Set rst = CurrentDb.OpenRecordset("tblMsnPers")
        rst.AddNew
        rst![MsnID] = Me.ID
        rst![EDIPI] = Me.PerSelect
        rst![NameStr] = DLookup("[NameStr]", "tblPersonnel", "[EDIPI] = '" & Me.PerSelect & "'")
        rst![PriAlt] = Me.cmbPriAlt
        rst![Errors] = DLookup("[ScrubErrors]", "tblPersonnel", "[EDIPI] = '" & Me.PerSelect & "'")
        rst![PT] = DLookup("[ScrubFitDate]", "tblPersonnel", "[EDIPI] = '" & Me.PerSelect & "'")
        rst![vRED] = DLookup("[ScrubvRED]", "tblPersonnel", "[EDIPI] = '" & Me.PerSelect & "'")
        rst![ISOPREP] = DLookup("[ISOPREP]", "tblPersonnel", "[EDIPI] = '" & Me.PerSelect & "'")
        rst![2760] = DLookup("[Scrub2760]", "tblPersonnel", "[EDIPI] = '" & Me.PerSelect & "'")
        rst![Checklist] = DLookup("[ScrubStatus]", "tblPersonnel", "[EDIPI] = '" & Me.PerSelect & "'")
        rst![IMR] = DLookup("[ScrubShots]", "tblPersonnel", "[EDIPI] = '" & Me.PerSelect & "'")
        rst![Review] = DLookup("[ReviewDate]", "tblPersonnel", "[EDIPI] = '" & Me.PerSelect & "'")
        rst.Update
        rst.Close
        Set rst = Nothing

And here is what my updated code is:

        DoCmd.SetWarnings False
        SqlStr = "INSERT INTO tblMsnPers " _
            & "(MsnID, EDIPI, PriAlt) VALUES " _
            & "('" & Me.ID & "', '" & Me.PerSelect & "', '" & Me.cmbPriAlt & "');"
        DoCmd.RunSQL SqlStr
        SqlStr2 = "UPDATE tblMsnPers INNER JOIN tblPersonnel ON tblMsnPers.EDIPI = tblPersonnel.EDIPI " _
            & "SET tblMsnPers.NameStr = [tblPersonnel].[NameStr], " _
            & "tblMsnPers.Errors = [tblPersonnel].[ScrubErrors], " _
            & "tblMsnPers.PT = [tblPersonnel].[ScrubFitDate], " _
            & "tblMsnPers.vRED = [tblPersonnel].[ScrubvRED], " _
            & "tblMsnPers.ISOPREP = [tblPersonnel].[ISOPREP], " _
            & "tblMsnPers.[2760] = [tblPersonnel].[Scrub2760], " _
            & "tblMsnPers.Checklist = [tblPersonnel].[ScrubStatus], " _
            & "tblMsnPers.IMR = [tblPersonnel].[ScrubShots], " _
            & "tblMsnPers.Review = [tblPersonnel].[ReviewDate], " _
            & "tblMsnPers.ATL1 = [tblPersonnel].[ATL1], " _
            & "tblMsnPers.SERE = [tblPersonnel].[SERE], " _
            & "tblMsnPers.CED = [tblPersonnel].[CED], " _
            & "tblMsnPers.GTCexp = [tblPersonnel].[ScrubGTC] " _
            & "WHERE ((tblMsnPers.MsnID = " & Me.ID & ") AND (tblMsnPers.EDIPI = '" & Me.PerSelect & "'));"
        DoCmd.RunSQL SqlStr2
        DoCmd.SetWarnings True

I can't help but feel like there's a better way to write the SQL string here because full disclosure, I have barely half a clue on what I'm doing here, being a student of reverse-engineering and Google-Fu. Is there a better way to write the SQL string here?

Copying all these values from one table to the other is sort of a red flag in itself. Or are they all default values that may be overwritten? — Andre, Aug 18 '20 at 13:01
Start by using parameters: [How do I use parameters in VBA in the different contexts in Microsoft Access?](https://stackoverflow.com/q/49509615/7296893). — Erik A, Aug 18 '20 at 13:55

score 1 · Accepted Answer · answered Aug 18 '20 at 23:13

I would use this

Dim rstPerson     As Recordset
Dim rst           As Recordset
Dim strSQL        As String

strSQL = "SELECT * from tblersonal where EDIPI = '" & Me.PerSelect & "'"
Set rstPer = CurrentDb.OpenRecordset(strSQL)
Set rst = CurrentDb.OpenRecordset("tblMsnPers")

With rst
  .AddNew
  !MnID = Me.ID
  !EDIPI = Me.PerSelect
  !NameStr = rstPer!NameStr
  !PriAlt = Me.cmbPriAlt
  !Errors = rstPer!ScrubErrors
  !PT = rstPer!ScrubFitDate
  !vRED = rstPer!ScrubvRED
  !ISOPREP = rstPer!ISOPREP
  ![2760] = rstPer!Scrub2760
  !Checklist = rstPer!ScrubStatus
  !IMR = rstPer!ScrubShots
  !Review = rstPer!ReviewDate
    
 .Update
 .Close
End With

This works well since:

All of the data type checking is done for you. (", strings, # dates, none for numbers)
You don't have messy concatenation.
You get parameter safe code (no sql injection - at least for update part).
It much less code. Far more readable.

And if you convert the data base to sql server, the above code will continue to work.

Is there a better way to write this Access SQL expression?

1 Answers1