Excel VBA Loop Stops Very Early

Question

I am a supreme novice with Excel VB, but I am trying to create a macro that compares a cell of a workbook to an entire column's worth of cell data and if it finds a match, copy and paste a range of data from one worksheet in a workbook to another with a couple caveats. The code below works well, but stops prematurely (about 250 of 2000 rows) into the loop.

The code is below

Sub copycomments()

Dim oldworkbook, newworkbook As Workbook
Set oldworkbook = Excel.Workbooks("Macro Experiment1.xlsm")
Set newworkbook = Excel.Workbooks("Macro Experiment2.xlsm")

Dim oldworksheet, newworksheet
Set ows = oldworkbook.Sheets("Sheet1")
Set nws = newworkbook.Sheets("Sheet1")

Dim oc, nc As Integer
nc = 2
oc = 2

Do While nws.Cells(nc, 5) <> ""
    Do While ows.Cells(oc, 5)
       If nws.Cells(nc, 5) = ows.Cells(oc, 5) Then
            If IsEmpty(nws.Range("T" & nc)) = True Then
                ows.Range("T" & oc & ":W" & oc).Copy _
                Destination:=nws.Range("T" & nc & ":W" & nc)
                Application.CutCopyMode = False
                Exit Do
            ElseIf IsEmpty(nws.Range("T" & nc)) = False Then
                ows.Range("W" & oc).Copy _
                Destination:=nws.Range("W" & nc)
                Application.CutCopyMode = False
                Exit Do
            End If
        End If
    oc = oc + 1
    Loop
oc = 2
nc = nc + 1
Loop

End Sub

I thought maybe the issue was the clipboard needed to be cleared after each copy and paste operation and that did seem to improve performance, but the loop still stopped working after about 300 rows.

Any advice is greatly appreciated!

Do While ows.Cells(oc, 5) expects a numeric value. Perhaps there is a line with text. Lot at the data in the row where your loop stops. BTW, I urge you to avoid omitting the property name. You mean Do While ows.Cells(oc, 5).Value (Value is the default property) but ows.Cells(oc, 5) is a range. Excel is pretty good at guessing what you mean but sometimes it fails and you will spend hours and hours looking for something that isn't there - the property name. — Variatus
If you are concerned about performance, it is more efficient to make the two ranges equal instead of copying/pasting. — Darrell H
It is a very nice try from a "supreme novice" :), just keep trying. So Variatus' comment is fine, While ows.Cells() is risky. See Val and IsNumeric, they handle strings, empty cells and 0 number a bit differently, select the right one. You check the same condition twice, one would be enough like If Isempty(...) Then <something> Else <something else>. For bool values you can simply omit = True as If fires in case of True and the IsEmpty returns bool (True or False). With the Exit Do you exit the inner loop after the first copy. Are you sure this is what you want? — AcsErno

Chris Dumas Chris Dumas · Accepted Answer · 2020-01-23T07:08:50

Thank you for the responses! Doing a more in depth look at why the macro was stopping, I noticed blank fields in ows.cells, which would’ve stopped the loop. I also cleaned up the exit do in the inner loop and the macro ran great. Got a couple smaller things to address, but all the major issues are resolved. Thanks again!!!

Excel VBA Loop Stops Very Early

1 Answers