Nested VBA loop crashes Excel

[daggg]

Public Sub abc123()
Dim EndRow As Long, I As Integer, J As Long, k As Long
I = 1
EndRow = Cells(Rows.Count, I).End(-4162).Row
Application.ScreenUpdating = False
For k = 1 To EndRow
For J = 1 To EndRow
If Cells(k, I + 8) = Cells(J, I) Then
Cells(J, I).Delete
Cells(J, I + 1).Delete
Cells(J, I + 2).Delete
Cells(J, I + 3).Delete
Cells(J, I + 4).Delete
Cells(J, I + 5).Delete
Cells(J, I + 6).Delete
Cells(J, I + 7).Delete
End If
Next
Next
Application.ScreenUpdating = True
End Sub


[FONT=&quot]The macro works on the sample but when I input my data of up to 20,000 rows the spreadsheet freezes and crashes excel.[/FONT]<img style="box-sizing: border-box; vertical-align: middle; border: 0px; color: rgb(73, 73, 73); font-family: "Gotham SSm", Helvetica, Arial, sans-serif; background-color: rgb(250, 250, 250);">

 

[daggg]

Re: Heeeeelp, please!

OK, here it is...
Need to enter data in columns a thru h the data can be all sorts. Then, we will enter data in column i. We need the macro to search column a for duplicate content that is in column i. Then, remove rows a thru h if the content in column a matches the content in column i.
The content in column i that needs to be found in column a could be anything at all. . .emails, address, house numbers, names... so really the entire contents of any cells in column i would need to be found in column a and then the rows a thru h of the corresponding duplicate would need to be removed.
Thanks.

 

[Michael M]

Re: Heeeeelp, please!

so, to clarify...
If the data in col A matches the data in col I on the same row...clear the contents from A to H of that row...looping through ALL rows.
Is that correct ???
If it isn't what you want, I'd suggest you post a SMALL sample of your data showing what you want done !!

 

[daggg]

Re: Heeeeelp, please!

If the data in col A matches the data in col I on the same row...delete that row...looping through ALL rows.
Thank you.

 

[daggg]

Re: Heeeeelp, please!

And there are 20000+ of rows, so that may be a problem...

 

[LockeGarmin]

Re: Heeeeelp, please!

As stated earlier your code is looping too many times. Instead of each row being evaluated once your code is evaluating each row 20,000 times. That's because you are using two For loops. I'm guessing you are thinking that by having two for loops next to each other you create code that loops once but increments two variables at the same time, but really what's happening your variable j gets incremented 20000+ times, then your variable k gets incremented once and j increments again 20000+ and this goes on until k gets incremented 20000+ times. Combine that with the fact that you are deleting cells and not a row at the same time and you have 20000 x 20000 x 8 = 3,200,000,000 delete operations which are relatively slow for Excel to do (that's worst case anyway, realistically it skip wherever column A and I are equal, but that is still a pretty big worst case scenario).

In your case I don't think you need k at all. This modification of your code will speed things up significantly (though you'll need to be patient because even with screen updating turned off it will take a while to complete 20000 delete operations) (Also, if this code still takes a while and your application screen goes "white" when you click somewhere, that doesn't mean Excel crashed. If you wait long enough the code will eventually finish):

[COLOR=#333333]Public Sub abc123()[/COLOR]
[COLOR=#333333]Dim EndRow As Long, J As Long[/COLOR]
[COLOR=#333333]EndRow = Cells(Rows.Count, 1).End(-4162).Row[/COLOR]
[COLOR=#333333]Application.ScreenUpdating = False[/COLOR]
[COLOR=#333333]For J = 1 To EndRow[/COLOR]
[COLOR=#333333]If Cells(J, 9) = Cells(J, 1) Then[/COLOR]
[COLOR=#333333]Range(Cells(J, 1), [/COLOR][COLOR=#333333]Cells(J, 8)).Delete[/COLOR]
[COLOR=#333333]End If[/COLOR]
[COLOR=#333333]Next[/COLOR]
[COLOR=#333333]Application.ScreenUpdating = True[/COLOR]
[COLOR=#333333]End Sub[/COLOR]

Now, this code will still be slow because you are deleting single rows one at a time, so you have 20,000 potential delete operations happening in code. It is a lot faster if you delete a batch of rows at the same time then to delete them one at a time. With that in mind the code below uses a building pattern that puts all of the rows into a single range variable and then calls the delete method on that variable to delete the rows all at once. You've gone back and forth saying you want to clear contents or you want to delete the rows entirely, so I commented out both options and put in a Select method instead. (I like to use select to look at what I'm about to delete just to make sure it is right). You can uncomment out whatever suits your case best and comment out or remove the select method. (I've also used a For Each loop instead of incrementing an integer with a For loop. This is just the style I like to use and there isn't anything really wrong with incrementing an integer like you were doing before)

Public Sub abc123()

Dim DataRange As Range
Dim DataRow As Range
Dim DeleteRange As Range

  Set DataRange = Range("H1", Cells(Rows.Count, "A").End(xlUp))
  
  For Each DataRow In DataRange.Rows
    If DataRow.Columns("I").Value2 = DataRow.Columns("A").Value2 Then
      If DeleteRange Is Nothing Then
        Set DeleteRange = DataRow
      Else
        Set DeleteRange = Excel.Union(DeleteRange, DataRow)
      End If
    End If
  Next
  
  DeleteRange.Select
  'DeleteRange.Delete xlShiftUp
  'DeleteRange.ClearContents
  
End Sub

Hope that all made sense! I can be a little long winded.

 

[Michael M]

Re: Heeeeelp, please!

with respect, I'd probably change the delete to Clearcontents
AND
If you really need to Delete..I'd start at the bottom of the data and work up
So this line

For J = 1 To EndRow

would be

For J = EndRow to 1 step -1

Public Sub abc123()
Dim EndRow As Long, J As Long
EndRow = Cells(Rows.Count, 1).End(-4162).Row
Application.ScreenUpdating = False
For J = 1 To EndRow
If Cells(J, 9) = Cells(J, 1) Then Range(Cells(J, 1), Cells(J, 8)).[color=red]ClearContents[/color]
Next J
Application.ScreenUpdating = True
End Sub

 

[LockeGarmin]

Re: Heeeeelp, please!

Oops, you're right, I should have been stepping backwards with

[COLOR=#333333]For J = EndRow to 1 step -1[/COLOR]

 

[Rick Rothstein]

Re: Heeeeelp, please!

OK, here it is...
Need to enter data in columns a thru h the data can be all sorts. Then, we will enter data in column i. We need the macro to search column a for duplicate content that is in column i. Then, remove rows a thru h if the content in column a matches the content in column i.
The content in column i that needs to be found in column a could be anything at all. . .emails, address, house numbers, names... so really the entire contents of any cells in column i would need to be found in column a and then the rows a thru h of the corresponding duplicate would need to be removed.

To be clear, a value in, say, cell I3 could match a value in, say, cell A123 and, if so, A123:H123 should be cleared (not deleted), correct?

If so, will the matches, when they occur, be exact or could the letter casing of one be different than the letter casing of the other?

You have 20,000 or so rows of data in columns A:H, but what about Column I... I am assuming it would be less (even much less) than 20,000 rows, correct?

 

[daggg]

Re: Heeeeelp, please!

If the content in column a matches the content in column i, then delete that row... Only this is important.
Thanks.

 

[Rick Rothstein]

Re: Heeeeelp, please!

If the content in column a matches the content in column i, then delete that row... Only this is important.

Maybe that is all that is important to you, but if you expect us to develop an efficient solution (or even any solution) for you, then you might want to answer the questions that were asked as they are important to me (and probably others) which is why I asked them. And just so you know, what you wrote above does not answer my first question.