Error Handling

tmyers

Well-known member
Local time
Today, 08:04
Joined
Sep 8, 2020
Messages
1,091
I am not the greatest when it comes to properly writing error handlers, but have been trying to improve on it and get them added into every module/code that could potentially fail. However, on this particular one I am not too sure how to correctly handle it.

Code:
    Set rstAdd = CurrentDb.OpenRecordset("Select * From tblContractorJob Where JobID = " & OldId & "")
    Set rst = rstAdd.Clone

    If rstAdd.RecordCount > 0 Then
        rstAdd.MoveLast
        rstAdd.MoveFirst
    End If
    Count = rstAdd.RecordCount
    For Item = 1 To Count
        With rstAdd
            .AddNew
            For Each fld In .Fields
                With fld
                    If .Attributes And dbAutoIncrField Then
                        ' Skip Autonumber or GUID field.
                    ElseIf .Name = "JobID" Then
                        ' Skip master/child field.
                        .Value = NewId
                    ElseIf .Name = "TypeID" Then
                        Set rstDrawingType = CurrentDb.OpenRecordset("Select * From tblFixtureTypes Where TypeID = " & rst!TypeID & " and JobID = " & OldId & "")
                        Set rstTypeID = CurrentDb.OpenRecordset("Select * From tblFixtureTypes Where TypeName = '" & rstDrawingType!TypeName & "' and JobID = " & NewId & "")
                        .Value = rstTypeID.Fields("TypeID").Value
                    Else
                        .Value = rst.Fields(.Name).Value
                    End If
                End With
            Next
            .Update
        End With
        rst.MoveNext
    Next

How would I correctly error handler this if there no record exist yet? I would still need the code to complete and not just out right exit. Or it might not need an handler and just another line or two on what to do if record count is zero. Could anyone offer some wisdom on this? This is also just a chunk of the code. The entire routine more or less does the same thing, just for different records. I can post the entire thing if need be.
 
Since I think it will quite possibly be needed, here is the entire block:
Code:
Public Sub CreateRevisions()

    Dim rst         As DAO.Recordset
    Dim rstAdd      As DAO.Recordset
    Dim rstCounts   As DAO.Recordset
    Dim rstCountsCopy  As DAO.Recordset
    Dim rstDrawingType As DAO.Recordset
    Dim fld         As DAO.Field
    Dim Count       As Integer
    Dim FieldCount  As Integer
    Dim Item        As Integer
    Dim ItemCount   As Integer
    Dim Bookmark    As Variant
    Dim OldId       As Long
    Dim NewId       As Long
    Dim strsql      As String
    Dim rstType As DAO.Recordset
   
   
    ' Copy parent record.
    Set rstAdd = Forms.JobQuote.RecordsetClone
    Set rst = rstAdd.Clone

   
    ' Move to current record.
    rst.Bookmark = Forms.JobQuote.Bookmark
    OldId = rst!JobID.Value
    With rstAdd
        .AddNew
        For Each fld In .Fields
            With fld
                If .Attributes And dbAutoIncrField Then
                    ' Skip Autonumber or GUID field.
                Else
                    .Value = rst.Fields(.Name).Value
                End If
            End With
        Next
        .Update
        ' Pick Id of the new record.
        .MoveLast
        NewId = !JobID.Value
    End With
    ' Store location of new record.
    Bookmark = rstAdd.Bookmark
   
    ' Copy products
   
    Set rstAdd = CurrentDb.OpenRecordset("Select * From tblProduct Where JobID = " & OldId & "")
    Set rst = rstAdd.Clone

    If rstAdd.RecordCount > 0 Then
        rstAdd.MoveLast
        rstAdd.MoveFirst
    End If
    Count = rstAdd.RecordCount
    For Item = 1 To Count
        With rstAdd
            .AddNew
            For Each fld In .Fields
                With fld
                    If .Attributes And dbAutoIncrField Then
                        ' Skip Autonumber or GUID field.
                    ElseIf .Name = "JobID" Then
                        ' Skip master/child field.
                        .Value = NewId
                    Else
                        .Value = rst.Fields(.Name).Value
                    End If
                End With
            Next
            .Update
        End With
        rst.MoveNext
    Next

    ' Copy fixture types
   
    Set rstAdd = CurrentDb.OpenRecordset("Select * From tblFixtureTypes Where JobID = " & OldId & "")
    Set rst = rstAdd.Clone

    If rstAdd.RecordCount > 0 Then
        rstAdd.MoveLast
        rstAdd.MoveFirst
    End If
    Count = rstAdd.RecordCount
    For Item = 1 To Count
        With rstAdd
            .AddNew
            For Each fld In .Fields
                With fld
                    If .Attributes And dbAutoIncrField Then
                        ' Skip Autonumber or GUID field.
                    ElseIf .Name = "JobID" Then
                        ' Skip master/child field.
                        .Value = NewId
                    Else
                        .Value = rst.Fields(.Name).Value
                    End If
                End With
            Next
            .Update
        End With
        rst.MoveNext
    Next
   
    ' Copy contractor counts
   
    Set rstAdd = CurrentDb.OpenRecordset("Select * From tblContractorJob Where JobID = " & OldId & "")
    Set rst = rstAdd.Clone

    If rstAdd.RecordCount > 0 Then
        rstAdd.MoveLast
        rstAdd.MoveFirst
    End If
    Count = rstAdd.RecordCount
    For Item = 1 To Count
        With rstAdd
            .AddNew
            For Each fld In .Fields
                With fld
                    If .Attributes And dbAutoIncrField Then
                        ' Skip Autonumber or GUID field.
                    ElseIf .Name = "JobID" Then
                        ' Skip master/child field.
                        .Value = NewId
                    ElseIf .Name = "TypeID" Then
                        Set rstDrawingType = CurrentDb.OpenRecordset("Select * From tblFixtureTypes Where TypeID = " & rst!TypeID & " and JobID = " & OldId & "")
                        Set rstTypeID = CurrentDb.OpenRecordset("Select * From tblFixtureTypes Where TypeName = '" & rstDrawingType!TypeName & "' and JobID = " & NewId & "")
                        .Value = rstTypeID.Fields("TypeID").Value
                    Else
                        .Value = rst.Fields(.Name).Value
                    End If
                End With
            Next
            .Update
        End With
        rst.MoveNext
    Next
   
    ' Copy drawings
    Set rstAdd = CurrentDb.OpenRecordset("Select * From tblDrawings Where JobID = " & OldId & "")

    Set rst = rstAdd.Clone

    If rstAdd.RecordCount > 0 Then
        rstAdd.MoveLast
        rstAdd.MoveFirst
    End If
    Count = rstAdd.RecordCount
    For Item = 1 To Count
        With rstAdd
            .AddNew
            For Each fld In .Fields
                With fld
                    If .Attributes And dbAutoIncrField Then
                        ' Skip Autonumber or GUID field.
                    ElseIf .Name = "JobID" Then
                        ' Skip master/child field.
                        .Value = NewId
                    Else
                        .Value = rst.Fields(.Name).Value
                    End If
                End With
            Next
            .Update
            rstAdd.Bookmark = rstAdd.LastModified
            ' update all child counts
            Set rstCounts = CurrentDb.OpenRecordset("Select * From tblDrawingFixtureType Where DrawingID = " & rst!DrawingID & " and JobID = " & OldId & "")
            Set rstCountsCopy = rstCounts.Clone
           
            If rstCounts.RecordCount > 0 Then
                rstCounts.MoveLast
                rstCounts.MoveFirst
            End If
            FieldCount = rstCounts.RecordCount
            For ItemCount = 1 To FieldCount
                With rstCounts
                    .AddNew
                    For Each fld In .Fields
                        With fld
                            If .Attributes And dbAutoIncrField Then
                                ' Skip Autonumber or GUID field.
                            ElseIf .Name = "JobID" Then
                                ' Skip master/child field.
                                .Value = NewId
                            ElseIf .Name = "DrawingID" Then
                                'Copy Old DrawingId
                                .Value = rstAdd!DrawingID
                            ElseIf .Name = "TypeID" Then
                                Set rstDrawingType = CurrentDb.OpenRecordset("Select * From tblFixtureTypes Where TypeID = " & rstCountsCopy!TypeID & " and JobID = " & OldId & "")
                           
                               
                                Set rstType = CurrentDb.OpenRecordset("Select * From tblFixtureTypes Where TypeName = '" & rstDrawingType!TypeName & "' and JobID = " & NewId & "")
                                .Value = rstType.Fields("TypeID").Value
                                rstDrawingType.Close
                                rstType.Close
                            Else
                                .Value = rstCountsCopy.Fields(.Name).Value
                            End If
                        End With
                    Next
                    .Update
                End With
                rstCountsCopy.MoveNext
            Next
       
            rstCountsCopy.Close
            rstCounts.Close
           
           
        End With
        rst.MoveNext
    Next
   
   
   
     
    ' Move to the new recordcopy.
    Forms.JobQuote.Bookmark = Bookmark
   
   
    Set fld = Nothing
    Set rstAdd = Nothing
    Set rst = Nothing
   
   
End Sub
 
at the top of the code ,put the goto
ON ERROR GOTO ErrName

then at the end of your code, put in the error block:
Code:
public sub MySub()
on error goto ErrName

'your code

exit sub   'stop your code

ErrName:     'begin error block
msgbox err.description, , err
end sub
 
at the top of the code ,put the goto
ON ERROR GOTO ErrName

then at the end of your code, put in the error block:
Code:
public sub MySub()
on error goto ErrName

'your code

exit sub   'stop your code

ErrName:     'begin error block
msgbox err.description, , err
end sub
How would I correctly write it to skip the block it failed on and move to the next? The way I had designed this, a lot of things get pretty wonky if this quits/fails completely part way through and typically results in corruption that I have to track down and remove.
 
I am not the greatest when it comes to properly writing error handlers, but have been trying to improve on it and get them added into every module/code that could potentially fail. However, on this particular one I am not too sure how to correctly handle it.

Code:
    Set rstAdd = CurrentDb.OpenRecordset("Select * From tblContractorJob Where JobID = " & OldId & "")
    Set rst = rstAdd.Clone

    If rstAdd.RecordCount > 0 Then
        rstAdd.MoveLast
        rstAdd.MoveFirst
    End If
    Count = rstAdd.RecordCount
    For Item = 1 To Count
        With rstAdd
            .AddNew
            For Each fld In .Fields
                With fld
                    If .Attributes And dbAutoIncrField Then
                        ' Skip Autonumber or GUID field.
                    ElseIf .Name = "JobID" Then
                        ' Skip master/child field.
                        .Value = NewId
                    ElseIf .Name = "TypeID" Then
                        Set rstDrawingType = CurrentDb.OpenRecordset("Select * From tblFixtureTypes Where TypeID = " & rst!TypeID & " and JobID = " & OldId & "")
                        Set rstTypeID = CurrentDb.OpenRecordset("Select * From tblFixtureTypes Where TypeName = '" & rstDrawingType!TypeName & "' and JobID = " & NewId & "")
                        .Value = rstTypeID.Fields("TypeID").Value
                    Else
                        .Value = rst.Fields(.Name).Value
                    End If
                End With
            Next
            .Update
        End With
        rst.MoveNext
    Next

How would I correctly error handler this if there no record exist yet? I would still need the code to complete and not just out right exit. Or it might not need an handler and just another line or two on what to do if record count is zero. Could anyone offer some wisdom on this? This is also just a chunk of the code. The entire routine more or less does the same thing, just for different records. I can post the entire thing if need be.
Hey, i would just add a if condition before your loop. You are already getting the count, so you can just check if the count is 0.

Code:
    Set rstAdd = CurrentDb.OpenRecordset("Select * From tblContractorJob Where JobID = " & OldId & "")
    Set rst = rstAdd.Clone

    If rstAdd.RecordCount > 0 Then
        rstAdd.MoveLast
        rstAdd.MoveFirst
    End If
    Count = rstAdd.RecordCount
    If Count > 0 Then
      For Item = 1 To Count
        With rstAdd
            .AddNew
            For Each fld In .Fields
                With fld
                    If .Attributes And dbAutoIncrField Then
                        ' Skip Autonumber or GUID field.
                    ElseIf .Name = "JobID" Then
                        ' Skip master/child field.
                        .Value = NewId
                    ElseIf .Name = "TypeID" Then
                        Set rstDrawingType = CurrentDb.OpenRecordset("Select * From tblFixtureTypes Where TypeID = " & rst!TypeID & " and JobID = " & OldId & "")
                        Set rstTypeID = CurrentDb.OpenRecordset("Select * From tblFixtureTypes Where TypeName = '" & rstDrawingType!TypeName & "' and JobID = " & NewId & "")
                        .Value = rstTypeID.Fields("TypeID").Value
                    Else
                        .Value = rst.Fields(.Name).Value
                    End If
                End With
            Next
            .Update
        End With
        rst.MoveNext
      Next
     End If
 
The normal method to check for no records is either

If rstAdd.RecordCount = 0 Then

or

If rstAdd.EOF or rstAdd.BOF Then

A record count in a DAO recordset is often wrong or can report -1 if you haven't issued a move last move first beforehand and if there are no records that will fail. However, if there are no records the record count does reliably return 0. Always.

Use the result of that check to set a flag(s) that you then use skip or run the next piece of code.
 
The normal method to check for no records is either

If rstAdd.RecordCount = 0 Then

or

If rstAdd.EOF or rstAdd.BOF Then

A record count in a DAO recordset is often wrong or can report -1 if you haven't issued a move last move first beforehand and if there are no records that will fail. However, if there are no records the record count does reliably return 0. Always.

Use the result of that check to set a flag(s) that you then use skip or run the next piece of code.
That is what I was thinking, but was unsure if that would be the correct method. This code (for me) is pretty complex and I am surprised I was able to cobble it together after reading various things.

I am learning however that when this code fails part way through, it causes all kinds of headaches with partially completed copies and such that almost always result in the record becoming unusable/corrupted.
 
Those were very helpful articles. Thank you jdraw!

I think I will take everyone's suggestion. I will add in the change Minty suggested as well as throw in an error handler just in case my variable rst returns as negative. I will set it to 0 then transfer the code back which should cause it to skip that block and then continue to the next.
 
I would never try to eat the elephant in one bite like you are doing. Makes chasing errors real hard. I think you could make this way easier. You have a bunch of sub procedures in my mind each one copies values to a table. I would build a Main routine and pass each subroutine the necessary parameters. Now you can check that each sub works fine. I would get rid of the looping and do a simple select into.
You could make this code so much cleaner that way and way more stable and easier to debug and error check.
 
I would never try to eat the elephant in one bite like you are doing. Makes chasing errors real hard. I think you could make this way easier. You have a bunch of sub procedures in my mind each one copies values to a table. I would build a Main routine and pass each subroutine the necessary parameters. Now you can check that each sub works fine. I would get rid of the looping and do a simple select into.
You could make this code so much cleaner that way and way more stable and easier to debug and error check.
I figured there were probably better ways to do this, but I am not versed enough to accomplish that. I was very close to breaking each section into its own module to be called however. Make the overall code easier to read and clean up. But how to improve it, that is lost on me.
 
Further. If you do what I said you make each sub return a yes/no if it is successful or not in its insert. If I had to do this I would think about setting this up as one big transaction so I can roll back if any piece fails.
 
Further. If you do what I said you make each sub return a yes/no if it is successful or not in its insert. If I had to do this I would think about setting this up as one big transaction so I can roll back if any piece fails.
I dont quite follow with the yes/no statement.
 
As a start the main routine
1. Main Routine Gets the old jobID
2. Main routine Calls function
InsertNewJob(OldJobID as Long) as Long
it inserts the new job and returns the new jobID back to the main. If the main gets this ID you were successful in the first insert
3. Main Routine then calls
InsertProducts(OldID as long, NewID as long) as boolean
Returns true if insert worked
4. Main Routine calls
InsertFixtures(OldID as long, NewId as long) as boolean
......
 
Ah got it.
Break each one apart, then add the boolean values to return true/false and if it returns false, something went wrong then I can gracefully exit and easily track where it broke.
 
I personally would get rid of the looping. More error prone and less efficient. Each subroutine simply does an insert into. I would do a select into. May need a little trick to insert the newID.
 
Yes, you have it. This is an example that I often "preach" to the newer members. "Divide and Conquer" worked for Julius Caesar so it ought to work for you.
 
I personally would get rid of the looping. More error prone and less efficient. Each subroutine simply does an insert into. I would do a select into. May need a little trick to insert the newID.
Just to make sure I 100% follow, rather than looping and doing that mess of code, just run a select/insert query instead. Use the code to gather all my variables then run a SQL block to do it rather than looping.
 
If interested this is how you do a transactions. If you fail anywhere you can roll back.
I like that transaction feature. That would make this work so much nicer since (at least in its current state) when it fails, it causes an entire host of problems. I wonder how I had never stumbled across it.
 

Users who are viewing this thread

Back
Top Bottom