I don't know what your data looks like, but the outer loop breaks on the second iteration with an empty DataSheet
, with the error you describe. That's a good thing, because cells1.Address
is $N$2:$N$1048576
on an empty sheet... but it doesn't matter, because the first empty cell meets the condition and exits the first inner loop.
But that's merely a symptom, not the real problem.
The second loop is iterating the exact same range as the first loop, but this time none of the empty cells meets the condition, and the loop goes on to iterate every single agonizing row on the worksheet.
And when that loop exits, the count2
loop variable reference is Nothing
- that's why the 2nd iteration explodes: [_Global|Worksheet].Range
accepts several different ways to specify a range of cells, but Nothing
is an illegal argument to give it; it raises an error, and execution abruptly stops.
What's happening? The VBA language specifications are relevant here:
When the <for-each-statement>
has finished executing, the value of <bound-variable-expression>
is the data value of the last element in <collection>
.
https://learn.microsoft.com/en-us/openspecs/microsoft_general_purpose_programming_languages/ms-vbal/b132463a-fd25-4143-8fc7-a443930e0651
It seems as per specs, both count1
and count2
should have a valid object reference when the loops exit. However it seems Microsoft's implementation of the VBA language specifications work differently. Here's a minimal repro example:
Public Sub Test()
Dim c As Collection
Set c = New Collection
c.Add New Collection 'any object will do
Dim o As Object
For Each o In c
'Exit For
Next
Debug.Print o Is Nothing
End Sub
Run this code once with Exit For
commented-out, then un-comment the statement and run it again. The "bound variable" of a For Each
loop that is iterating an object collection will be Nothing
if the loop runs to completion.
That means if the first inner loop ran to completion, you would be having the same error here:
Set Cells2 = Range(Count1, Range("N2").End(xlDown))
Because Count1
would be Nothing
then. On an empty sheet, Count1
is pointing to $N$2
at this point, so Cells2
gets to be $N$2:$N$1048576
.
When the second inner loop runs to completion, Count2
is Nothing
, and since the loop condition is only looking at Count1
...
Loop Until Count1 Is Nothing
...the second iteration of the outer loop blows up when Count2
is passed as an argument to _Global.Range
, which fails with error 1004:
Set Cells1 = Range(Count2, Range("N2").End(xlDown))
I suppose a band-aid solution could be to verify whether Count2 Is Nothing
before using it, although that's exactly the same as changing the exit condition to check for both Count1
and Count2
:
Loop Until Count1 Is Nothing Or Count2 Is Nothing
...and I don't think that's the right solution. In fact I'm not sure it's a solution.
I can't be 100% sure because I haven't spent too much time trying to work out how Count2
not being Nothing
affects the outer loop (plus I didn't have any data to toy with), but I think this might just happen to be doing the same thing:
Dim interestingCells As Range
Set interestingCells = DataSheet.Range("N2:N" & DataSheet.Rows.Count).End(xlUp)
Dim cell As Range
For Each cell In interestingCells
If cell.Value < 1 And cell.Offset(3).Value < 1 Then
cell.Offset(0, 20).Value = cell.Offset(0, -12).Value
ElseIf cell.Value > 1 And cell.Offset(3).Value > 1 Then
cell.Offset(0, 21).Value = cell.Offset(0, -12).Value
cell.Offset(1, 22).Value = cell.Offset(1).Value
End If
Next
I honestly think the real solution is to take a step back and re-assess exactly what this loop means to be doing, scrap the old code, and rewrite it from scratch. Anything else would add complexity to an already incredibly hard-to-follow piece of code. Other than the recycled For Each
loop variables, the row offsets are particularly confusing: a comment detailing why the next row needs to be affected would be in order - note that such logic probably requires the data to be sorted in a particular way... which might be a bad assumption to make, i.e. a disastrous bug waiting to show its ugly head. Also note that if the iterated range happens to span the entire column, then cell.Offset(n)
will fail for any positive value of n
when cell
is in the very last row, and for any value of n
greater than the number of rows left under it if it's near the very last row on the worksheet.
Also note the .End(xlUp)
logic to get the last row/cell with data: with that, an empty sheet will not run a single iteration.
Lastly, note the explicitly qualified Range
call, so that it resolves to DataSheet.Range
rather than _Global.Range
- this is one of the very few times I've seen error 1004 thrown by _Global.Range
where the root of the issue wasn't the use of unqualified Range
calls implicitly referring to whatever the ActiveSheet
is.