For Each only working when ran twice (1 Viewer)

[tmyers]

I am confused by a problem I am having. I have a module that is just deleting rows based on criteria and one block seems to only work when the module is ran by itself but when it is ran during the whole process, it doesnt do anything.

Here is the full module:

Public Sub Criteria()

Dim wb                  As Workbook:    Set wb = ThisWorkbook
Dim ws                  As Worksheet:   Set ws = wb.Sheets("Config")
Dim OrderNum            As Long:        OrderNum = ws.Range("B9")
Dim ws2                 As Worksheet:   Set ws2 = wb.Sheets(OrderNum & " Report")
Dim ws3                 As Worksheet:   Set ws3 = wb.Sheets("Job Info")
Dim Customer            As String
Dim Job                 As String
Dim PO                  As String
Dim c                   As Range
Dim arry                As Variant
Dim arry2               As Variant
Dim lastrow             As Long
Dim i                   As Long
Dim RowsToDrop          As Variant
Dim num                 As Variant


With ws3.Range("A2:Z1000")
    Set c = .Find(OrderNum, LookIn:=xlValues)
    If c Is Nothing Then
        MsgBox ("No job information entered in Job Info. No filters or header applied to order " & ws.Range("B9").Value)
        Exit Sub
    End If
        Customer = c.Offset(1, 0)
        Job = c.Offset(2, 0)
        PO = c.Offset(3, 0)
        arry = c.Offset(4, 0)
        arry2 = c.Offset(5, 0)
End With

ws2.Range("C8").Value = Customer
ws2.Range("C9").Value = Job
ws2.Range("C10").Value = PO


RowsToDrop = Split(arry, ",")

lastrow = ws2.Cells(Rows.Count, 4).End(xlUp).Row

For Each num In RowsToDrop
    For i = lastrow To 12 Step -1
        If Cells(i, 1).Value Like num & "*" Then
            Rows(i).Delete
        End If
    Next i
Next num

RowsToDrop = Split(arry2, ",")

lastrow = ws2.Cells(Rows.Count, 4).End(xlUp).Row

For Each num In RowsToDrop
    For i = lastrow To 12 Step -1
        If Cells(i, 5).Value = num Then
            Rows(i).Delete
        End If
    Next i
Next num


End Sub

The section that seems to be selective on when it runs is:

RowsToDrop = Split(arry2, ",")

lastrow = ws2.Cells(Rows.Count, 4).End(xlUp).Row

For Each num In RowsToDrop
    For i = lastrow To 12 Step -1
        If Cells(i, 5).Value = num Then
            Rows(i).Delete
        End If
    Next i
Next num

In the above block, num is a string whereas in the For Each above it, it is a number, but I dont think that should matter since it is a variant. Any idea why this only wants to work some of the time?

 

[Isaac]

Do you have any variable names that are re-used, or I mean, the same variable name is used outside the scope of this procedure and could be confused? Like maybe lastrow? (I just say that as it's common).

That's all I could think of, as I don't have much experience in using Variants, also I usually declare arrays like this:
Dim variablename() as String 'or whatever
But you're right, declared as variant and then assigned a value using Split ought to work, I think.

I also have never looped through an array using a collection-type syntax, I usually do:

For x = 0 to ubound(array)

Next x

But have no idea if that matters here or will spark any thing in your thought process that's helpful.

Your code and your approach have come a long way, nice job

 

[tmyers]

I spend a lot of time doing it now a days, so I can actually practice and get better. Also have gotten much better at wording questions for Google searches

 

[tmyers]

I went through and made all the variables unique across the entire workbook but that block still doesn't delete rows unless I run the module by itself. Maybe its a problem with not explicitly naming the sheet...

 

[Gasman]

Walk through the code.

 

[tmyers]

The variable is getting the values correct but for some reason the criteria for the IF statement is never met and the rows do not get deleted even if column 5 on the given row has that status. I changed it from .value = to .value Like and even added a wild card and it still never seems to enter the If statement.

 

[The_Doc_Man]

one block seems to only work when the module is ran by itself but when it is ran during the whole process, it doesnt do anything.

This behavior is characteristic of something that has a side effect, i.e. it uses something globally that shouldn't be global (or vice versa) - which COULD include that the process has some effect on some of the declared items.

The variable is getting the values correct but for some reason the criteria for the IF statement is never met and the rows do not get deleted even if column 5 on the given row has that status. I changed it from .value = to .value Like and even added a wild card and it still never seems to enter the If statement.

If you put a breakpoint on the IF statement, don't only look for the values related to "num" but look for anything else that you DIM'd that should have a value but doesn't, or that shouldn't have a value but does.

In the above block, num is a string whereas in the For Each above it, it is a number, but I dont think that should matter since it is a variant. Any idea why this only wants to work some of the time?

A variant number (or, for that matter, any OTHER type of number) doesn't compare very well with a string. That "num" in the loop is bound to a loop index which means you can't change its type but the comparison wouldn't matter. In that case I don't think the data type dynamically changes because the general rule is to find a number at the beginning of the string. E.g. if the string is "3dognight" then its value in that comparison is 3 even though you have a string behind it.

 

[Isaac]

I went through and made all the variables unique across the entire workbook but that block still doesn't delete rows unless I run the module by itself. Maybe its a problem with not explicitly naming the sheet...

good point - i would DEFINITELY add something qualifying to rows()

absolutely that's going to behave differently depending on active sheet.

 

[Isaac]

are you intending to change C's offset even if the IF statement does not trigger?

 

[tmyers]

are you intending to change C's offset even if the IF statement does not trigger?

I am. That section looks through the sheet to find the order number then offsets to grab the rest of the information for the variables. If the IF statement for c is ran, it exits the entire module as there is no information to use. I added in the sheet reference to rows.delete but it unfortunately did not correct the problem.

Here is the procedure that calls this module, maybe your sharper eyes can find if something here is causing it:

Option Explicit

Private Sub CommandButton1_Click()

Dim Mypath      As String:          Mypath = Application.ThisWorkbook.FullName
Dim pFile       As String
Dim pFolder     As String
Dim wb          As Workbook:        Set wb = ThisWorkbook
Dim ws          As Worksheet:       Set ws = wb.Sheets("Config")
Dim f           As Object
Dim varItem     As Variant
Dim wShell      As Object
Dim wait        As Boolean:         wait = True
Dim Errorcode   As Long
Dim pExe        As String
Dim OrderNum    As String
Dim OrdersToRun As Variant
Dim Order       As Variant


ws.Range("B3").Value = Mypath

If IsEmpty(ws.Range("B4")) Then

    Set f = Application.FileDialog(msoFileDialogFilePicker)
        f.AllowMultiSelect = False
       
        If f.Show Then
            For Each varItem In f.SelectedItems
                pFile = Dir(varItem)
                'pFolder = Left(varItem, Len(varItem) - Len(pFile))
            Next
            ws.Range("B4").Value = pFile
        End If
End If

OrderNum = Application.InputBox("Please enter all SE order numbers. Separate all numbers by a comma with no spaces")

OrdersToRun = Split(OrderNum, ",")

For Each Order In OrdersToRun

    ws.Range("B9").Value = Order
    pExe = ws.Range("B4").Value
    Set wShell = VBA.CreateObject("WScript.Shell")
   
    Errorcode = wShell.Run(pExe, 0, wait)
   
    If Errorcode <> 0 Then
        MsgBox "An error has occured", vbOKOnly
        Set wShell = Nothing
        Exit Sub
    End If
    Set wShell = Nothing
   
    Call Criteria
    Call FormatReport
   
Next Order

End Sub

 

[tmyers]

I am an idiot and this is why I really should use Option Explicit on every single module as well as better variables names, I had misspelled some of the variables. Once I noticed that and correct it, everything works now.

While I am at it though, in regards to the last code segment in post #10, is there a better way to handle that shell command? It seems really iffy on when it wants to work and not error out. The file it is trying to execute is stored on a file server not local to the computer which is why I think it has problems.

 

[Isaac]

Glad it's working but to me this is a problem. I've never become a super-super-expert on variable scope, mostly because I simply avoid the issue by making things quite unique. (Which is actually part of my overall scheme to make variable names informative and steering rather than short and cryptic - EXCEPT in simple routines where there is no doubt as to interpretation, like I also use WB and WS a lot).

But to me this is a problem - these 3 variables are duplicated both inside and outside the procedure, which could confuse your code and make you have to become a serious expert in variable scope and passing in order to continue doing this and avoid a problem byref byval.

wb

ws

OrderNum

edit: I suppose maybe this is no problem if you aren't trying to pass any of them into a proc. I may have gotten ahead of myself here...may disregard

 

[Isaac]

While I am at it though, in regards to the last code segment in post #10, is there a better way to handle that shell command? It seems really iffy on when it wants to work and not error out. The file it is trying to execute is stored on a file server not local to the computer which is why I think it has problems.

In some cases thisworkbook.followhyperlink can be wonderfully simplistic and still work. You can do that if you don't mind the security prompt - and there may be a way to avoid that or (please forget I ever said this: use sendkeys just ONE time in your life ha ha).

Also look into ShellExecute, many people swear by this although I have used it relatively little, as I usually know which file type I want to open.

question: what file type do you want to open

 

[Isaac]

I am an idiot and this is why I really should use Option Explicit on every single module as well as better variables names, I had misspelled some of the variables. Once I noticed that and correct it, everything works now.

While I am at it though, in regards to the last code segment in post #10, is there a better way to handle that shell command? It seems really iffy on when it wants to work and not error out. The file it is trying to execute is stored on a file server not local to the computer which is why I think it has problems.

you know how to auto-put option explicit on all new modules from now on right? vba options 'require variable declaration' ? just in case u forget

 

[tmyers]

Yup I made that change so it is auto added every time

question: what file type do you want to open

It is a frozen python script in a .exe format. I changed how I was running the shell a little bit and think I worked out all the "jank" by adding in more quotation marks to fully and nicely wrap the file path. I also have to use wScript.Shell as I have to wait for the process to finish before continuing and I read that you could not get the functionality from the simple Shell command.

Glad it's working but to me this is a problem. I've never become a super-super-expert on variable scope, mostly because I simply avoid the issue by making things quite unique. (Which is actually part of my overall scheme to make variable names informative and steering rather than short and cryptic - EXCEPT in simple routines where there is no doubt as to interpretation, like I also use WB and WS a lot).

But to me this is a problem - these 3 variables are duplicated both inside and outside the procedure, which could confuse your code and make you have to become a serious expert in variable scope and passing in order to continue doing this and avoid a problem byref byval.

wb

ws

OrderNum

edit: I suppose maybe this is no problem if you aren't trying to pass any of them into a proc. I may have gotten ahead of myself here...may disregard

I do generally try and be more obvious with my variable names as it helps so much when you come back to it months later but did get a little lazy on this one but have since gone back and fixed it.

I also changed OrderNum and now only declare it in the main procedure and just pass it to the other subroutines rather than declaring it each time. This also could all be avoided by making all of this one routine, but I was always told it was best that each action should be its own module so I try to do that when it makes sense to.

 

[Isaac]

Coolio, dont know much about python scripts.
Some people also use ShellAndWait, I just tried it the other day actually, but my excel crashed, so I ditched it.
Believe I've used it successfully before, this time I just didn't feel like figuring out why it was crashing.
top answer: https://stackoverflow.com/questions/1374433/shellexecuteex-in-vba

 

[Isaac]

but I was always told it was best that each action should be its own module so I try to do that when it makes sense to.

yes, kind of a subjective continuum, but definitely an excellent rule of thumb. pays dividends as the program grows.