Code Review - Looking for constructive feedback

R

RocketRod

New member
Local time
Yesterday, 21:42
Joined
Apr 24, 2007
Messages
8
I have setup a small test database with which has three tables:
RsrcHoursWorked - Contains information pertaining to hours worked by a resource in a given week (ADMUSER2_FINDATESSTART_DATE).
Rsrcrate- Contains all of the available rates and there effective date for each resource.
RsrcRateByFinPrd - A temporary table that is recreated when the user click the Test button. After excuting the code below, the table will contain information from the RsrcHoursWorked and Rsrcrate table, which aligns the hours worked by a resource with the hourly rate that was effective when the work was performed.

This is my first time working with databases and was looking for some constructive feedback on my code, or suggestions on how to improve it. I attached a copy of the test database below.


Code: 
Private Sub btnTest_Click()
On Error GoTo Err_btnTest_Click

    Dim db As Database
    Dim rsTarget As DAO.Recordset
    Dim strSQL As String
    Dim sSQL_DEL As String
    Dim strSQL_SEL As String
    Dim strSQL_UPDATE As String
    Dim vStr As Variant
    
    DoCmd.SetWarnings True
    DoCmd.Hourglass True
    
    'Open connection to current Access database
    Set db = CurrentDb()
        
    ' Delete query
    sSQL_DEL = "DELETE * FROM RsrcRateByFinPrd"
    
    ' Run query to ensure the table is empty
    DoCmd.RunSQL (sSQL_DEL)
 
    ' Append query to populate RsrcRateByFinPrd table in DESC order
    strSQL = "INSERT INTO RsrcRateByFinPrd ( Rsrc_ID, ADMUSER2_FINDATESSTART_DATE ) " & _
             "SELECT DISTINCT [RsrcHoursWorked].[RSRC_ID], [RsrcHoursWorked].[ADMUSER2_FINDATESSTART_DATE] " & _
             "FROM RsrcHoursWorked " & _
             "ORDER BY [RsrcHoursWorked].[RSRC_ID], [RsrcHoursWorked].[ADMUSER2_FINDATESSTART_DATE] DESC;"
        
    ' Run query to populate RsrcRateByFinPrd table
    DoCmd.RunSQL (strSQL)

    Set rsTarget = db.OpenRecordset("RsrcRateByFinPrd")

    rsTarget.MoveFirst
    
    Do Until rsTarget.EOF
        Set vCurID = rsTarget.Fields(0)
        ' Select query to retrieve RsrcRate info in DESC order
        strSQL_SEL = "SELECT [Rsrcrate].[RSRC_RATE_ID], [Rsrcrate].[RSRC_ID], [Rsrcrate].[START_DATE], [Rsrcrate].[COST_PER_QTY], [Rsrcrate].[COST_PER_QTY2], [Rsrcrate].[COST_PER_QTY3], [Rsrcrate].[COST_PER_QTY4], [Rsrcrate].[COST_PER_QTY5]" & vbCrLf & _
                    "FROM Rsrcrate " & vbCrLf & _
                    "WHERE [Rsrcrate].[RSRC_ID]=" & rsTarget.Fields(1) & " " & vbCrLf & _
                    "ORDER BY [Rsrcrate].[START_DATE] DESC;"
     
        Dim rsFind As DAO.Recordset
        
        Set rsFind = db.OpenRecordset(strSQL_SEL, , dbSQLPassThrough)
       
        rsFind.MoveFirst
        Dim vCost As Variant
        Dim vCost2 As String
        
        If rsFind.RecordCount = 0 Then

            strSQL_UPDATE = "UPDATE RsrcRateByFinPrd" & vbCrLf & _
                            "SET RSRC_RATE_ID =" & rsFind.Fields(0) & vbCrLf & _
                            ", COST_PER_QTY =" & 0 & vbCrLf & _
                            ", COST_PER_QTY2 =" & 0 & vbCrLf & _
                            "WHERE [RsrcRateByFinPrd].[ID]=" & vCurID & ";"
            DoCmd.RunSQL (strSQL_UPDATE)

        ElseIf rsFind.RecordCount = 1 Then
            ' Check for NULL value
            If IsNull(rsFind.Fields(3)) Then
                vCost = 0
            Else
                vCost = rsFind.Fields(3)
            End If

            ' Check for NULL value
            If IsNull(rsFind.Fields(4)) Then
                vCost2 = 0
            Else
                vCost2 = rsFind.Fields(4)
            End If
                   
            strSQL_UPDATE = "UPDATE RsrcRateByFinPrd" & vbCrLf & _
                            "SET RSRC_RATE_ID =" & rsFind.Fields(0) & vbCrLf & _
                            ", COST_PER_QTY =" & vCost & vbCrLf & _
                            ", COST_PER_QTY2 =" & vCost2 & vbCrLf & _
                            "WHERE [RsrcRateByFinPrd].[ID]=" & vCurID & ";"
            'Run query
            DoCmd.RunSQL (strSQL_UPDATE)
           
        Else
            Dim iCnt, iMax As Integer
            Dim bFound As Boolean
            Dim sMsg As String
            bFound = False
            iCnt = 0
            iMax = rsFind.RecordCount
            
            While iCnt < iMax
                 If rsFind.Fields(2) <= rsTarget.Fields(3) And bFound = False Then
                    ' Check for NULL value
                    If IsNull(rsFind.Fields(3)) Then
                       vCost = 0
                    Else
                        vCost = rsFind.Fields(3)
                    End If
                    
                    ' Check for NULL value
                    If IsNull(rsFind.Fields(4)) Then
                        vCost2 = 0
                    Else
                        vCost2 = rsFind.Fields(4)
                    End If
                    
                   strSQL_UPDATE = "UPDATE RsrcRateByFinPrd" & vbCrLf & _
                                   "SET RSRC_RATE_ID = " & rsFind.Fields(0) & vbCrLf & _
                                   ", COST_PER_QTY =" & vCost & vbCrLf & _
                                   ", COST_PER_QTY2 =" & vCost2 & vbCrLf & _
                                   "WHERE [RsrcRateByFinPrd].[ID]=" & vCurID & ";"

                   'Run query
                    DoCmd.RunSQL (strSQL_UPDATE)

                    ' Set to true since we found the correct rate
                    bFound = True
                    
                    ' Set to iMax to end While loop
                    iCnt = iMax
                    
                ElseIf iCnt < iMax Then
                    iCnt = iCnt + 1

                    'Get next record if we haven't found our rate and there are additional records
                    rsFind.MoveNext
                ElseIf iCnt = iMax Then
                    If bFound = False Then
                       strSQL_UPDATE = "UPDATE RsrcRateByFinPrd" & vbCrLf & _
                                       "SET RSRC_RATE_ID = " & rsFind.Fields(0) & vbCrLf & _
                                       ", COST_PER_QTY =" & 0 & vbCrLf & _
                                       ", COST_PER_QTY2 =" & 0 & vbCrLf & _
                                       "WHERE [RsrcRateByFinPrd].[ID]=" & vCurID & ";"
                        'Run query
                        DoCmd.RunSQL (strSQL_UPDATE)
                    End If
                End If
            Wend
            
        End If
    rsTarget.MoveNext
Loop
    rsTarget.Close
    rsFind.Close
    
    Set rsTarget = Nothing
    Set rsFind = Nothing
    
    DoCmd.SetWarnings False
    DoCmd.Hourglass False
    
Exit_btnTest_Click:
    Exit Sub

Err_btnTest_Click:
    MsgBox Err.Description
    Resume Exit_btnTest_Click

End Sub

Thanks in advance,
Rod
 

Attachments

In no particular order:-

Database Observations
---------------------
Your tables have repeating groups - this is generally considered to be bad practice.
Rsrcrate - COST_PER_QTY, COST_PER_QTY2, COST_PER_QTY3, (etc)

Read up on "Database Normalisation (Normalization)" for more information.

In a correctly designed & structured database, you would be able to achieve the result you desire without temporary tables or code - just a query.

Is your DB environment a single user, or multi-user environment? In a multi-user environment, consider what would happen if two users press this button at the same time.

In the three tables, you have defined RSRC_ID and RSRC_RATE_ID (and others) as Number, Double. However, only integer values are present. Whilst in a small application/table this matters little, you are using an extra 4 bytes per field per row per table, that might be unnecessary.

Code Observations
-----------------
Without compromising the statement above - code should not be required to perform this task - below are some suggestions/observations relating to the code.

(I suspect from the look of the structure that you are on the receiving end of an export of sorts from another db or application, so you might not be in absolute control of the structure???)

Option Explicit is a good thing to use - it forces the explicit declaration of all variables in the module, which can help to reduce coding errors (i.e. incorrectly typing/spelling a variable at some point, causing the creation of a new variable, where you intended to use an existing one - Option Explicit would pick this up by insisting you declare it, which should prompt you to at least think about it.)

Naming conventions serve best when adhered to. i.e. try to avoid mixing them up...
strSQL, sSQL_DEL
Dim vCost As Variant
Dim vCost2 As String (????)

You have declared several strings that you don't really need - one strSQL would suffice.

I prefer to declare all my variables at the top of the module, in two blocks - object / non-object.
I find this helps to ensure I release all my object variables at the end of the procedure, rather than having to scour the whole procedure to hunt them down. This helps with good housekeeping.

Declaring multiple variables on the one line does not behave as I suspect you might believe...

Dim iCnt, iMax As Integer
will dimension iCnt as a variant,
and iMax as an Integer

Dim iCnt as Integer, iMax As Integer
will dimension them both as integers

To see this for yourself, compare results of VarType(iCnt), VarType(vCost), and VarType(iMax) while your code is running (in break mode).

Do you have good reason to use dbSQLPassThrough for rsFind ?

I doubt vbCRLFs are required in you strSQL's - a space or (existing) comma should be adequate.

e.g.
Code: 
strSQL_UPDATE = "UPDATE RsrcRateByFinPrd" & vbCrLf & _
                            "SET RSRC_RATE_ID =" & rsFind.Fields(0) & _
                            ", COST_PER_QTY =" & 0 & _
                            ", COST_PER_QTY2 =" & 0 & _
                            " WHERE [RsrcRateByFinPrd].[ID]=" & vCurID & ";"

Also, you have that same block of code in four places. Consider finding a way to have it in only one place.

You could use the NZ() function instead of this construct:-

The online help on the NZ() function is worth reading.

Code: 
                    ' Check for NULL value
'                    If IsNull(rsFind.Fields(3)) Then
'                       vCost = 0
'                    Else
'                        vCost = rsFind.Fields(3)
'                    End If

' could be written more simply as 
       vCost = nz(rsFind.Fields(3),0)


docmd.setwarnings = True
at the outset, and = False at the end, results in users having to click OK hundreds of times. You may want to set it to False at the beginning, and True at the end.

You might also consider setting the hourglass to false at the top of your error handler. Otherwise users would still get the hourglas over the error box, and might not realise they can clcik OK, and don't have to wait (forever....) for the hourglass to go away.

It is not necessary to set a counter and a max to iterate over a recordset (or a collection for that matter). (iCnt, Imax)

I'm guessing you didn't encounter a case where rsfind.recordcount = 0 ??
1) rsFind.MoveFirst will fail if recordcount = 0. (because there is no first record)
2) rsFind.Fields(0) is also invalid in this situation (no current record)


Whilst I still recommend using a correct structure and a query to achieve this, if you must use code....

Here's my crack at re-structuring and tidying it up a bit...
Please do your own testing (if you are going to use it) to ensure it meets your needs - i.e. all care, no responsibility on my part...

Code: 
Option Explicit

Private Sub btnTest_Click()
On Error GoTo Err_btnTest_Click

    'Object Variables
    Dim db As DAO.Database
    Dim rsTarget As DAO.Recordset
    Dim rsFind As DAO.Recordset
    
    'Non Object Variables
    Dim strSQL As String
    Dim lngCurID As Long
    Dim vCost As Variant
    Dim strCost2 As String
    Dim bFound As Boolean
    Dim lngRateID As Long
    
    DoCmd.SetWarnings False
    DoCmd.Hourglass True
    
    'Open connection to current Access database
    Set db = CurrentDb()
        
    ' Delete query
    strSQL = "DELETE * FROM RsrcRateByFinPrd"
    
    ' Run query to ensure the table is empty
    DoCmd.RunSQL (strSQL)
 
    ' Append query to populate RsrcRateByFinPrd table in DESC order
    strSQL = "INSERT INTO RsrcRateByFinPrd ( Rsrc_ID, ADMUSER2_FINDATESSTART_DATE ) " & _
             "SELECT DISTINCT [RsrcHoursWorked].[RSRC_ID], [RsrcHoursWorked].[ADMUSER2_FINDATESSTART_DATE] " & _
             "FROM RsrcHoursWorked " & _
             "ORDER BY [RsrcHoursWorked].[RSRC_ID], [RsrcHoursWorked].[ADMUSER2_FINDATESSTART_DATE] DESC;"
        
    ' Run query to populate RsrcRateByFinPrd table
    DoCmd.RunSQL (strSQL)

    Set rsTarget = db.OpenRecordset("RsrcRateByFinPrd")

    rsTarget.MoveFirst
    
    Do Until rsTarget.EOF
        lngCurID = rsTarget.Fields(0)
        ' Select query to retrieve RsrcRate info in DESC order
        strSQL = "SELECT [Rsrcrate].[RSRC_RATE_ID], [Rsrcrate].[RSRC_ID], [Rsrcrate].[START_DATE], [Rsrcrate].[COST_PER_QTY], [Rsrcrate].[COST_PER_QTY2]" & _
                    " FROM Rsrcrate" & _
                    " WHERE [Rsrcrate].[RSRC_ID]=" & rsTarget.Fields(1) & _
                    " ORDER BY [Rsrcrate].[START_DATE] DESC;"
        
        Set rsFind = db.OpenRecordset(strSQL)
       
          
         If (rsFind.BOF And rsFind.EOF) Then
            'No matching records
            vCost = 0
            strCost2 = 0
            lngRateID = 0
         Else
            '1 or more matching records
            rsFind.MoveFirst
            If rsFind.RecordCount = 1 Then
               ' Check for NULL value
                vCost = Nz(rsFind.Fields(3), 0)
                strCost2 = Nz(rsFind.Fields(4), 0)
                lngRateID = rsFind.Fields(0)
            Else
               'set default values - will be used only if none found
               vCost = 0
               strCost2 = 0
               lngRateID = 0
               bFound = False
               Do While Not rsFind.EOF And Not bFound
                  If rsFind.Fields(2) <= rsTarget.Fields(3) Then
                      ' Check for NULL value
                      vCost = Nz(rsFind.Fields(3), 0)
                      strCost2 = Nz(rsFind.Fields(4), 0)
                      lngRateID = rsFind.Fields(0)
                      ' Set to true since we found the correct rate
                      bFound = True
                  Else
                     'Get next record if we haven't found our rate and there are additional records
                     rsFind.MoveNext
                  End If
                  
               Loop
            End If
        End If
        rsFind.Close
        Set rsFind = Nothing
        
        strSQL = "UPDATE RsrcRateByFinPrd" & vbCrLf & _
                        "SET RSRC_RATE_ID =" & lngRateID & _
                        ", COST_PER_QTY =" & vCost & _
                        ", COST_PER_QTY2 =" & strCost2 & _
                        " WHERE [RsrcRateByFinPrd].[ID]=" & lngCurID & ";"
        DoCmd.RunSQL (strSQL)

        rsTarget.MoveNext
   Loop

Exit_btnTest_Click:

    On Error Resume Next
    'Homeward Bound - don't care if the below fails
    rsTarget.Close
    rsFind.Close
    
    Set rsTarget = Nothing
    Set rsFind = Nothing
    
    DoCmd.SetWarnings True
    DoCmd.Hourglass False
    
    Exit Sub

Err_btnTest_Click:
    DoCmd.Hourglass False
    MsgBox "Error # " & Err.Number & vbCrLf & Err.Description, "Ooops!"
    Resume Exit_btnTest_Click
    Resume
End Sub


HTH

Regards

John.
 
Thanks for the insightful and very constructive feedback. It is very helpful as its been over three years since I have done any developing, which was in VC++, and this is my first experience with VBA and SQL.

Also after I posted this I have realized this could be down with queries only, but would there be any performance impacts doing additional queries versus iterating recordsets?

john471 said:
In no particular order:-

Database Observations
---------------------
Your tables have repeating groups - this is generally considered to be bad practice.
Rsrcrate - COST_PER_QTY, COST_PER_QTY2, COST_PER_QTY3, (etc)

Read up on "Database Normalisation (Normalization)" for more information.

In a correctly designed & structured database, you would be able to achieve the result you desire without temporary tables or code - just a query.
Click to expand...
This is a test, in the production version the RsrcHoursWorked & RsrcRateByFinPrd will be queries and the Rsrcrate is a table in another database. Also the reason RsrcRateByFinPrd is a table is that it by other queries to generate a dozen or so reports and I only want to run the query to run once. I also planned to normalize the data within my area of control.

john471 said:
Is your DB environment a single user, or multi-user environment? In a multi-user environment, consider what would happen if two users press this button at the same time.
Click to expand...
I considered this, but this is a single user application.

john471 said:
In the three tables, you have defined RSRC_ID and RSRC_RATE_ID (and others) as Number, Double. However, only integer values are present. Whilst in a small application/table this matters little, you are using an extra 4 bytes per field per row per table, that might be unnecessary.
Click to expand...
I got a little sloppy setting up the test database, but it will be corrected in the production version.

Code Observations
-----------------
john471 said:
Without compromising the statement above - code should not be required to perform this task - below are some suggestions/observations relating to the code.
Click to expand...
True, but I am limited by the way the external database I pulling information from is structured and my knowledge of SQL.


john471 said:
(I suspect from the look of the structure that you are on the receiving end of an export of sorts from another db or application, so you might not be in absolute control of the structure???)
Click to expand...
Painfully true. Add to this that I inherited this project and the previous developers designed the application around a set of requirements that have changed drastically and the design was not built with any future changes in mind.

john471 said:
Option Explicit is a good thing to use - it forces the explicit declaration of all variables in the module, which can help to reduce coding errors (i.e. incorrectly typing/spelling a variable at some point, causing the creation of a new variable, where you intended to use an existing one - Option Explicit would pick this up by insisting you declare it, which should prompt you to at least think about it.)

Naming conventions serve best when adhered to. i.e. try to avoid mixing them up...
strSQL, sSQL_DEL
Dim vCost As Variant
Dim vCost2 As String (????)

You have declared several strings that you don't really need - one strSQL would suffice.

I prefer to declare all my variables at the top of the module, in two blocks - object / non-object.
I find this helps to ensure I release all my object variables at the end of the procedure, rather than having to scour the whole procedure to hunt them down. This helps with good housekeeping.
Click to expand...
Oversight on my part.


john471 said:
Declaring multiple variables on the one line does not behave as I suspect you might believe...

Dim iCnt, iMax As Integer
will dimension iCnt as a variant,
and iMax as an Integer

Dim iCnt as Integer, iMax As Integer
will dimension them both as integers

To see this for yourself, compare results of VarType(iCnt), VarType(vCost), and VarType(iMax) while your code is running (in break mode).
Click to expand...
I was not aware that VBA did this.

john471 said:
Do you have good reason to use dbSQLPassThrough for rsFind ?
Click to expand...
No

john471 said:
I doubt vbCRLFs are required in you strSQL's - a space or (existing) comma should be adequate.

e.g.
Code: 
strSQL_UPDATE = "UPDATE RsrcRateByFinPrd" & vbCrLf & _
                            "SET RSRC_RATE_ID =" & rsFind.Fields(0) & _
                            ", COST_PER_QTY =" & 0 & _
                            ", COST_PER_QTY2 =" & 0 & _
                            " WHERE [RsrcRateByFinPrd].[ID]=" & vCurID & ";"

Also, you have that same block of code in four places. Consider finding a way to have it in only one place.
Click to expand...
This was only done so I could read the SQL when displayed in a MsgBox.

john471 said:
You could use the NZ() function instead of this construct:-

The online help on the NZ() function is worth reading.

Code: 
                    ' Check for NULL value
'                    If IsNull(rsFind.Fields(3)) Then
'                       vCost = 0
'                    Else
'                        vCost = rsFind.Fields(3)
'                    End If

' could be written more simply as 
       vCost = nz(rsFind.Fields(3),0)
Click to expand...
I new vba had to have a method for this but I am just learning.

john471 said:
docmd.setwarnings = True
at the outset, and = False at the end, results in users having to click OK hundreds of times. You may want to set it to False at the beginning, and True at the end.

You might also consider setting the hourglass to false at the top of your error handler. Otherwise users would still get the hourglas over the error box, and might not realise they can clcik OK, and don't have to wait (forever....) for the hourglass to go away.

It is not necessary to set a counter and a max to iterate over a recordset (or a collection for that matter). (iCnt, Imax)
Click to expand...
Good suggestion(s)


john471 said:
I'm guessing you didn't encounter a case where rsfind.recordcount = 0 ??
1) rsFind.MoveFirst will fail if recordcount = 0. (because there is no first record)
2) rsFind.Fields(0) is also invalid in this situation (no current record)
Click to expand...
True. There will always be at least one record.

john471 said:
Whilst I still recommend using a correct structure and a query to achieve this, if you must use code....
Click to expand...
Since I have posted this I have since realized this could be down with queries only, but would there be any performance impacts doing additional queries versus iterating recordsets with code?

Here's my crack at re-structuring and tidying it up a bit...
Please do your own testing (if you are going to use it) to ensure it meets your needs - i.e. all care, no responsibility on my part...

Code: 
Option Explicit

Private Sub btnTest_Click()
On Error GoTo Err_btnTest_Click

    'Object Variables
    Dim db As DAO.Database
    Dim rsTarget As DAO.Recordset
    Dim rsFind As DAO.Recordset
    
    'Non Object Variables
    Dim strSQL As String
    Dim lngCurID As Long
    Dim vCost As Variant
    Dim strCost2 As String
    Dim bFound As Boolean
    Dim lngRateID As Long
    
    DoCmd.SetWarnings False
    DoCmd.Hourglass True
    
    'Open connection to current Access database
    Set db = CurrentDb()
        
    ' Delete query
    strSQL = "DELETE * FROM RsrcRateByFinPrd"
    
    ' Run query to ensure the table is empty
    DoCmd.RunSQL (strSQL)
 
    ' Append query to populate RsrcRateByFinPrd table in DESC order
    strSQL = "INSERT INTO RsrcRateByFinPrd ( Rsrc_ID, ADMUSER2_FINDATESSTART_DATE ) " & _
             "SELECT DISTINCT [RsrcHoursWorked].[RSRC_ID], [RsrcHoursWorked].[ADMUSER2_FINDATESSTART_DATE] " & _
             "FROM RsrcHoursWorked " & _
             "ORDER BY [RsrcHoursWorked].[RSRC_ID], [RsrcHoursWorked].[ADMUSER2_FINDATESSTART_DATE] DESC;"
        
    ' Run query to populate RsrcRateByFinPrd table
    DoCmd.RunSQL (strSQL)

    Set rsTarget = db.OpenRecordset("RsrcRateByFinPrd")

    rsTarget.MoveFirst
    
    Do Until rsTarget.EOF
        lngCurID = rsTarget.Fields(0)
        ' Select query to retrieve RsrcRate info in DESC order
        strSQL = "SELECT [Rsrcrate].[RSRC_RATE_ID], [Rsrcrate].[RSRC_ID], [Rsrcrate].[START_DATE], [Rsrcrate].[COST_PER_QTY], [Rsrcrate].[COST_PER_QTY2]" & _
                    " FROM Rsrcrate" & _
                    " WHERE [Rsrcrate].[RSRC_ID]=" & rsTarget.Fields(1) & _
                    " ORDER BY [Rsrcrate].[START_DATE] DESC;"
        
        Set rsFind = db.OpenRecordset(strSQL)
       
          
         If (rsFind.BOF And rsFind.EOF) Then
            'No matching records
            vCost = 0
            strCost2 = 0
            lngRateID = 0
         Else
            '1 or more matching records
            rsFind.MoveFirst
            If rsFind.RecordCount = 1 Then
               ' Check for NULL value
                vCost = Nz(rsFind.Fields(3), 0)
                strCost2 = Nz(rsFind.Fields(4), 0)
                lngRateID = rsFind.Fields(0)
            Else
               'set default values - will be used only if none found
               vCost = 0
               strCost2 = 0
               lngRateID = 0
               bFound = False
               Do While Not rsFind.EOF And Not bFound
                  If rsFind.Fields(2) <= rsTarget.Fields(3) Then
                      ' Check for NULL value
                      vCost = Nz(rsFind.Fields(3), 0)
                      strCost2 = Nz(rsFind.Fields(4), 0)
                      lngRateID = rsFind.Fields(0)
                      ' Set to true since we found the correct rate
                      bFound = True
                  Else
                     'Get next record if we haven't found our rate and there are additional records
                     rsFind.MoveNext
                  End If
                  
               Loop
            End If
        End If
        rsFind.Close
        Set rsFind = Nothing
        
        strSQL = "UPDATE RsrcRateByFinPrd" & vbCrLf & _
                        "SET RSRC_RATE_ID =" & lngRateID & _
                        ", COST_PER_QTY =" & vCost & _
                        ", COST_PER_QTY2 =" & strCost2 & _
                        " WHERE [RsrcRateByFinPrd].[ID]=" & lngCurID & ";"
        DoCmd.RunSQL (strSQL)

        rsTarget.MoveNext
   Loop

Exit_btnTest_Click:

    On Error Resume Next
    'Homeward Bound - don't care if the below fails
    rsTarget.Close
    rsFind.Close
    
    Set rsTarget = Nothing
    Set rsFind = Nothing
    
    DoCmd.SetWarnings True
    DoCmd.Hourglass False
    
    Exit Sub

Err_btnTest_Click:
    DoCmd.Hourglass False
    MsgBox "Error # " & Err.Number & vbCrLf & Err.Description, "Ooops!"
    Resume Exit_btnTest_Click
    Resume
End Sub


HTH

Regards

John.[/QUOTE]

Thanks,
Rod
 
Thinking more about it this morning, also realised that you don't neeed SQL to update the "target" recordset - just use

rstarget.edit
rstarget.fields("RSRC_RATE_ID").value = lngRateID
rstarget.fields("COST_PER_QTY").value = vCost
rstarget.fields("COST_PER_QTY2").value = vCost2
rstarget.update

(rstarget.movenext)
 
You must log in or register to reply here.

Users who are viewing this thread

Total: 1 (members: 0, guests: 1)
Back
Top Bottom