Object variable or with block variable not set, with my On Error GoTo

Question

So I have code that loops through every row on one sheet (DataWS) and uses other sheets to find strings, if the strings are not found then I want the code to go to wherever the On Error tells it to. So if the part is not found on a worksheet, then go to (somewhere). If the strings are found on a worksheet then the DataWS is updated or the found variable is used later on in the code to reference the row it was found on in the other sheets.

Sub MM011_to_Data()

Dim Dlr As Long, Dlc As Long, x As Long
Dim DataWS As String, MMWS As String, OmitWS As String


    On Error GoTo DataSheetInvalid
        Sheets("Data").Activate
        DataWS = Sheets("Data").Name
    On Error GoTo 0

    On Error GoTo MM011SheetInvalid
        MMWS = Sheets("MM011").Name
    On Error GoTo 0

    On Error GoTo OmitSheetInvalid
        OmitWS = Sheets("Omit Parts").Name
    On Error GoTo 0

'********************BELOW ARE VARIABLES IN THE DATA SHEET********************

Dim DStatuscol As Integer, DSupplySourcecol As Integer, DPOcol As Integer, DPOIcol As Integer, DPRcol As Integer
Dim DPLOcol As Integer, DWillShipcol As Integer, DSOLIcol As Integer, DRelPNcol As Integer

DStatuscol = Sheets(DataWS).Rows(1).Find("Status", lookat:=xlWhole).Column
DSupplySourcecol = Sheets(DataWS).Rows(1).Find("Supply Source", lookat:=xlWhole).Column
DPOcol = Sheets(DataWS).Rows(1).Find("PO#", lookat:=xlWhole).Column
DPOIcol = Sheets(DataWS).Rows(1).Find("PO Item", lookat:=xlWhole).Column
DPRcol = Sheets(DataWS).Rows(1).Find("PR#", lookat:=xlWhole).Column
DPLOcol = Sheets(DataWS).Rows(1).Find("PLO", lookat:=xlWhole).Column
DWillShipcol = Sheets(DataWS).Rows(1).Find("Will Ship", lookat:=xlWhole).Column
DSOLIcol = Sheets(DataWS).Rows(1).Find("CC SO/LI", lookat:=xlWhole).Column
DRelPNcol = Sheets(DataWS).Rows(1).Find("RELEASED PN", lookat:=xlWhole).Column


'********************BELOW ARE VARIABLES IN THE MM011 SHEET********************

Dim MMSupplySourcecol As Integer, MMInvcol As Integer, MMPOcol As Integer, MMPOIcol As Integer, MMPLOcol As Integer, MMPRcol As Integer
Dim MMPOShipDatecol As Integer, MMStatuscol As Integer, MMPNConcatcol As Integer

MMPNConcatcol = Sheets(MMWS).Rows(1).Find("PN Concatenate", lookat:=xlWhole).Column
MMSupplySourcecol = Sheets(MMWS).Rows(1).Find("Supply Source", lookat:=xlWhole).Column
MMInvcol = Sheets(MMWS).Rows(1).Find("Inventory Qty", lookat:=xlWhole).Column
MMPOcol = Sheets(MMWS).Rows(1).Find("PO Number", lookat:=xlWhole).Column
MMPOIcol = Sheets(MMWS).Rows(1).Find("PO Item", lookat:=xlWhole).Column
MMPLOcol = Sheets(MMWS).Rows(1).Find("Supply Work Order", lookat:=xlWhole).Column
MMPRcol = Sheets(MMWS).Rows(1).Find("Requisition Number", lookat:=xlWhole).Column
MMPOShipDatecol = Sheets(MMWS).Rows(1).Find("Supply Delivery Calendar Date", lookat:=xlWhole).Column


'********************BELOW ARE THE VARIABLES IN THE OMIT SHEET********************

Dim OPNcol As Integer, OPNrow As Long

OPNcol = Sheets(OmitWS).Rows(1).Find("Part Number", lookat:=xlWhole).Column

'********************LOOP TO UPDATE THE DATA SHEET********************

Dim DConcatPNSO As String, MMConcatrow As String
Dim SupplySource As String, PO_Number As String, PO_Item As String, PLO_WO As String, Requisition_Number As String, Supply_Del_Date As String
Dim DPartNumber As String

Dlr = Sheets(DataWS).Cells(Rows.Count, 1).End(xlUp).Row
Dlc = Sheets(DataWS).Cells(1, Columns.Count).End(xlToLeft).Column


    For x = 2 To Dlr

'DConcatPNSO is the variable for the Data WS which concatenates the Released PN and the SO LI
        DConcatPNSO = Sheets(DataWS).Cells(x, DRelPNcol) & " " & Sheets(DataWS).Cells(x, DSOLIcol)
        DPartNumber = Sheets(DataWS).Cells(x, DRelPNcol)

'If there is no sales order then skip and say no sales order (NSO)
        If Sheets(DataWS).Cells(x, DSOLIcol) = "" Then
            Sheets(DataWS).Cells(x, DSupplySourcecol) = "NSO"
        GoTo Updated
        End If

'Checks to see if the part is on the Omit list
    On Error GoTo NotOmit
        OPNrow = Sheets(OmitWS).Columns(OPNcol).Find(What:=DPartNumber, lookat:=xlWhole).Row
    On Error GoTo 0

    If OPNrow > 0 Then
        Cells(x, DSupplySourcecol) = "Omit"
    GoTo Updated
    End If

NotOmit:

        On Error GoTo PN_SO_NotFound
            MMConcatrow = Sheets(MMWS).Columns(MMPNConcatcol).Find(DConcatPNSO, lookat:=xlWhole).Row
        On Error GoTo 0

'Hold all of the info from cells into variables
            SupplySource = Sheets(MMWS).Cells(MMConcatrow, MMSupplySourcecol)
            PO_Number = Sheets(MMWS).Cells(MMConcatrow, MMPOcol)
            PO_Item = Sheets(MMWS).Cells(MMConcatrow, MMPOIcol)
            PLO_WO = Sheets(MMWS).Cells(MMConcatrow, MMPLOcol)
            Requisition_Number = Sheets(MMWS).Cells(MMConcatrow, MMPRcol)
            Supply_Del_Date = Format(Sheets(MMWS).Cells(MMConcatrow, MMPOShipDatecol), "MM/DD/YYYY")

'If the Supply Source is PO then do the below
                If SupplySource = "PO" Then
                    Sheets(DataWS).Cells(x, DStatuscol) = SupplySource & " " & Sheets(MMWS).Cells(MMConcatrow, MMPOcol) _
                    & " " & Sheets(MMWS).Cells(MMConcatrow, MMPOIcol) & " " & Format(Sheets(MMWS).Cells(MMConcatrow, MMPOShipDatecol), "MM/DD/YYYY")
                    Sheets(DataWS).Cells(x, DSupplySourcecol) = SupplySource
                    Sheets(DataWS).Cells(x, DPOcol) = Sheets(MMWS).Cells(MMConcatrow, MMPOcol)
                    Sheets(DataWS).Cells(x, DPOIcol) = Sheets(MMWS).Cells(MMConcatrow, MMPOIcol)
                    Sheets(DataWS).Cells(x, DWillShipcol) = Format(Sheets(MMWS).Cells(MMConcatrow, MMPOShipDatecol), "MM/DD/YYYY")
                GoTo Updated
                End If

'If the Supply Source is PLO then do the below
                If SupplySource = "PLO" Then
                    Sheets(DataWS).Cells(x, DStatuscol) = SupplySource & " " & Sheets(MMWS).Cells(MMConcatrow, MMPLOcol)
                    Sheets(DataWS).Cells(x, DSupplySourcecol) = SupplySource
                    Sheets(DataWS).Cells(x, DPLOcol) = Sheets(MMWS).Cells(MMConcatrow, MMPLOcol)
                GoTo Updated
                End If

'If the Supply Source is PR then do the below
                If SupplySource = "PR" Then
                    Sheets(DataWS).Cells(x, DStatuscol) = SupplySource & " " & Sheets(MMWS).Cells(MMConcatrow, MMPRcol)
                    Sheets(DataWS).Cells(x, DSupplySourcecol) = SupplySource
                    Sheets(DataWS).Cells(x, DPRcol) = Sheets(MMWS).Cells(MMConcatrow, MMPRcol)
                GoTo Updated
                End If

'If the Supply Source is BPL then do the below
                If SupplySource = "BPL" Then
                    Sheets(DataWS).Cells(x, DStatuscol) = SupplySource & " " & "Borrow Payback Loan"
                    Sheets(DataWS).Cells(x, DSupplySourcecol) = SupplySource
                GoTo Updated
                End If

'If the Supply Source is QA then do the below
                If SupplySource = "QA" Then
                    Sheets(DataWS).Cells(x, DStatuscol) = SupplySource & ", PO is " & Sheets(MMWS).Cells(MMConcatrow, MMPOcol) _
                    & " " & Sheets(MMWS).Cells(MMConcatrow, MMPOIcol) & " " & Format(Sheets(MMWS).Cells(MMConcatrow, MMPOShipDatecol), "MM/DD/YYYY")
                    Sheets(DataWS).Cells(x, DSupplySourcecol) = SupplySource
                    Sheets(DataWS).Cells(x, DPRcol) = Sheets(MMWS).Cells(MMConcatrow, MMPRcol)
                GoTo Updated
                End If


Updated:

    Next x

Exit Sub

'****************ERROR HANDLING****************

DataSheetInvalid:
MsgBox "The worksheet with the MS2 All Open Order Report should be titled ""Data""." & vbCr & vbCr & "Please rename the worksheet and restart this sub.", vbCritical, "Worksheet Name"
Exit Sub

MM011SheetInvalid:
MsgBox "The worksheet with Cognos (MM011) data should be titled ""MM011""." & vbCr & vbCr & "Please rename the worksheet and restart this sub.", vbCritical, "Worksheet Name"
Exit Sub

OmitSheetInvalid:
MsgBox "The worksheet with Omit Parts data should be titled ""Omit Parts""." & vbCr & vbCr & "Please rename the worksheet and restart this sub.", vbCritical, "Worksheet Name"
Exit Sub

PN_SO_NotFound:
Sheets(DataWS).Cells(x, DSupplySourcecol) = "N/A"
GoTo Updated

End Sub

So within this code, the part that keeps getting the Run-Time Error is either one of these On Error handlers:

'Checks to see if the part is on the Omit list
    On Error GoTo NotOmit
        OPNrow = Sheets(OmitWS).Columns(OPNcol).Find(What:=DPartNumber, lookat:=xlWhole).Row
    On Error GoTo 0

    If OPNrow > 0 Then
        Cells(x, DSupplySourcecol) = "Omit"
    GoTo Updated
    End If

NotOmit:

        On Error GoTo PN_SO_NotFound
            MMConcatrow = Sheets(MMWS).Columns(MMPNConcatcol).Find(DConcatPNSO, lookat:=xlWhole).Row
        On Error GoTo 0

On the first loop they both work, but when it comes to any loop afterwards, I get the error.

To fix these errors I used this code:

 Set OPNrng = Sheets(OmitWS).Columns(OPNcol).Find(What:=DPartNumber, lookat:=xlWhole)
    If OPNrng Is Nothing Then
        GoTo NotOmit
    Else
        OPNrow = OPNrng.Row
    End If

If OPNrow > 0 Then
    Cells(x, DSupplySourcecol) = "Omit"
GoTo Updated
End If

NotOmit:

    Set MMConcatrng = Sheets(MMWS).Columns(MMPNConcatcol).Find(DConcatPNSO, lookat:=xlWhole)
    If MMConcatrng Is Nothing Then
    GoTo PN_SO_NotFound
    Else
    MMConcatrow = MMConcatrng.Row
    End If

Is there a reason why the "On Error GoTo" syntax was not working?? I cannot figure out why. Also, is there a better way to do it besides what I have??

If I can explain anything else further please let me know, I tried to give you the whole picture as well as the code separated out where I am having these issues.

*Is there a better way* - **YES!!**. Instead of the spaghetti style `GoTo` statements, use functions and built-in VBA (and programming in general) methods to error check for you. For example, [WorksheetExists](https://stackoverflow.com/questions/6688131/test-or-check-if-sheet-exists/#6688482) can solve your first couple of lines. You can also break the code into component procedures and have the flow control in one procedure. Your code would be a lot easier to read and maintain. — Scott Holtzman, Nov 21 '17 at 22:39
Sounds good. I'm about 4 months into VBA and don't know a lot of the syntax still but I will remember that WorksheetExists method. As for breaking it up into component procedures, what do you mean by that? Lastly, by spaghetti style, I assume that is meaning how it kind of jumps around? I will keep all of this in mind going forward, thanks. — Adije, Nov 22 '17 at 02:34

score 3 · Accepted Answer · answered Nov 21 '17 at 22:50

PN_SO_NotFound:
Sheets(DataWS).Cells(x, DSupplySourcecol) = "N/A"
GoTo Updated

This. You're in an error-handling subroutine - the VBA runtime is in an error state, and you're GoTo-jumping all over the place.

When execution goes to the Updated label after hitting PN_SO_NotFound, VBA is still in an error state because you never reset it. So as far as it understands your code, the Updated label is just another part of your error-handling subroutine.

And while it thinks it's still handling a runtime error, it's not going to heed any On Error statements it encounters, because it's already in an error state.

Replace GoTo Updated with Resume Updated and, well, honestly, fingers crossed, it should "work".

But definitely consider some SERIOUS refactoring here. Spaghetti code like this isn't getting any easier to debug anytime soon without a serious series of refactorings: you'll know you're done when there's a grand total of 0 GoTo jumps and all procedures fit into a single screen, each doing one single thing.

Thanks for the comment/answer/advice and excuse my ignorance but for the refactoring, how would be the best way to do that? Also you're saying all of my programming should fit into one screen? Or I should have a sub for each individual task and then one sub calling them? This is probably my infancy with programming and just not understanding the best way/logic to do things but thanks for the help, I'll keep everything you said in mind in the future. — Adije, Nov 22 '17 at 02:51
@Adije in general you want to name a procedure after its purpose: if it's hard to find a good name for a procedure, it's likely because that procedure has too many responsibilities. To know where to break procedures, you need to take the bigger problem and decompose it into smaller problems; each smaller problem has its own concerns, and these even-smaller problems could be more, smaller `Private` procedures. Do that and you'll quickly move from "God-procedure" to structured procedural code, and eventually to object-oriented. Enjoy the ride! — Mathieu Guindon, Nov 22 '17 at 02:56

score 1 · Answer 2 · answered Nov 22 '17 at 07:13

1

Instead of using only On Error GoTo 0 use the combination of On Error GoTo 0 : On Error Goto -1 to reset both the exception handler and the error state.

answered Nov 22 '17 at 07:13

Sujoy

1,051
13
26

Object variable or with block variable not set, with my On Error GoTo

2 Answers2