2

How do I fix the Not Responding error in Excel VBA?

My code is perfect, I'm not getting any error but after I run it, I will see NOT RESPONDING top of Excel VBA and Excel doesn't respond anymore.

I am writing vba code to find the match numbers between two sheets: Sheet1 holding 5000 numbers and Sheet2 holding 4500 numbers. When I run it, my code run it for 10 secs then excel freezing on me and not responding msg appear top of the screen.

When I compary only 100 numbers between two sheets, it works perfectly and not responding message does not appear. It only occurs when I compare 10000 numbers together or larger amount of numbers.

Once again this code is working perfectly, when I am comparing only 100 numbers. But when I try to do larger numbers between two sheets, "not responding" appears and excel freezes. Here is my code and I did try screenupdate or others, but nothing helpes. I will really appreciate, if someone help me out.

     Dim Sheet1rows As Long, Sheet2rows As Long, a as long, j as long
      j = 1
      a = 1
      Sheet1rows = Sheets("Sheet1").UsedRange.Rows.Count + 1
      Sheet2rows = Sheets("Sheet2").UsedRange.Rows.Count + 1

     For i = a To Sheet1rows
       For ii = a To Sheet2rows

    'this one is copying between to sheets

        If Sheets("Sheet1").Cells(i, 1) = Sheets("Sheet2").Cells(ii, 1) Then
         Sheets("Sheet3").Range("A" & j) = Sheets("Sheet1").Cells(i, 1)
        j = j + 1
       next ii
     next i
peterh
  • 11,875
  • 18
  • 85
  • 108
user3795861
  • 61
  • 1
  • 3
  • 10
  • This doesn't really fix the code any, but you should be able to stop code that is "not responding" in excel by spamming the esc key. This interrupts the program and you should be able to click the stop button. This could be useful while troubleshooting and fixing the actual issue. – BeaumontTaz Jul 08 '14 at 00:40
  • 1
    Clearly your code is not perfect as 1. It causes not responding and 2. You are not utilising built in functions as @hnk has suggested. If you want to use loops then add a couple of `DoEvents` inside the loop to allow other processes to work, but a better solution is to follow @hnk's advice. – Nick.Mc Jul 08 '14 at 00:54
  • 2
    To compare 10000 rows like this you are actually doing **one-hundred million** comparisons. I bet if you let it sit for 30 minutes or 60 minutes or so, it will ultimately finish the process. It is just taking a *very long time* to do so, during which the application appears unresponsive. I suspect there is a better way to do what you're trying to do, but without knowing **what** you're trying to do, it is difficult to provide much more assistance than that @hnk has already offered. – David Zemens Jul 08 '14 at 02:21
  • What are you planning on doing with these numbers? Because there may be a non-VBA fix for this problem pending what you intent on doing with these (and more precisely what format you need it in.) Also, is there anything else on either of these sheets? – BeaumontTaz Jul 08 '14 at 02:29
  • well, @user3795861, there is a limited amount of 'help' me or anyone can provide here. You need to understand this annoying thing about algorithms. Algorithm >> Language + Implementation + Optimization ---> What that means is, that you can optimize the living daylights out of bubble sort in assembly with SIMD, but... – hnk Jul 08 '14 at 03:30
  • hi. do have a look @ the pseudocode in the solution. Might help speed things up somewhat – hnk Jul 08 '14 at 03:58

5 Answers5

5

You've gotten some pretty stellar answers to make your code better already. I just want to add one thing.

If you just want to keep the program from FREEZING, then add

DoEvents

inside each loop on its own line.

I'd also suggest adding

Application.StatusBar = "Updating" & (i)

so that you can at least see if it is working.

Tanner
  • 548
  • 8
  • 20
  • Sure. Adding an Application.DoEvents would certainly make things more interactive but in this particular case would also make it drag forever because of the additional Event checking overhead (though it could be done every 1000 iterations or so, that way it still stays interactive). But v. good point. – hnk Jul 08 '14 at 12:36
  • 1
    I agree with you. This won't improve the FUNCTION a whole lot. It would still take DAYS. However, this will cause it not to freeze. It could be done every so often, like you say. – Tanner Jul 08 '14 at 14:14
4

This is a terribly inefficient piece of code. So it's getting stuck on longer operations.

While ideally you should rewrite it completely using

VariantVariable = RangeVariable

and then doing the opposite to put data back onto the sheet (and hence minimizing the number of times your code accesses the spreadsheet), a few quick fixes for now would include:

Application.Calculation = xlCalculationManual
Application.ScreenUpdating = False
Application.EnableEvents = False

' Your code goes here

Application.Calculation = xlCalculationAutomatic
Application.ScreenUpdating = True
Application.EnableEvents = True

This might help speed things up a bit.

Proposed algo (pseudocode): If what you are looking to do is Compare columns in Sheet 1 and Sheet 2 and then Copy the Matching numbers to Sheet 3, you are doing something that's at best O(n^2) in complexity, and looking at the implementation could easily be even more. In either case, here is the pseudocode on how you may proceed.

Assume both columns have 'n' elements.

Dim LHS as Variant, RHS as Variant
LHS = RangeOfColumnInSheet1
RHS = RangeOfColumnInSheet2

Sort -> LHS
Sort -> RHS
' You may use dictionary structures if you please

Merge Join -> LHS + RHS ------> MergedList

Scan(MergedList) to find out non-matching numbers -> UniqueList

OutputRange = UniqueList

Assuming Quicksort for sorting, we are talking of O(n log n) for sorting on average and O(n) for merge-join-filtering.

Then I would store my sorted arrays as cached columns elsewhere so that all further operations will take place in O(n). Why is this good? For several reasons.

  1. this might not be the most efficient way to go about it, but it sure beats O(n^2).

  2. To give you a sense of comparison. If your current system is taking 1 second to go through columns of length 100 elements each, it will take 11+ days to go through two columns of 100,000 elements each.

  3. Similarly, if the proposed system above after pre-sorting takes 1 second for 100 elements, it'll only take about 16+ minutes for 100,000 elements.

As mentioned in the comment, you can optimize the living daylights out of bubblesort, but...

Edit: More discussion on the algo on this page: How to intersect two sorted integer arrays without duplicates?

Community
  • 1
  • 1
hnk
  • 2,216
  • 1
  • 13
  • 18
  • 1
    *This is a terribly inefficient piece of code.* Indeed. It's difficult to imagine why one would need to compare every cell in a range to *each* of every cell in another range on a different sheet. Perhaps there's a reason, but I suspect that OP has made this procedure a lot more complicated than it needs to be. – David Zemens Jul 08 '14 at 02:19
  • i did used application.screenupdating events, i just forgot to add it here and it doesnt work, its a samething make no change – user3795861 Jul 08 '14 at 02:24
  • well, @user3795861, there is a limited amount of 'help' me or anyone can provide here. You need to understand this annoying thing about algorithms. Algorithm >> Language + Implementation + Optimization ---> What that means is, that you can optimize the living daylights out of bubble sort in assembly with SIMD, but... – hnk Jul 08 '14 at 03:29
  • 3
    If you aren't interested in listening to advice then I suggest you don't waste time asking questions. – Nick.Mc Jul 08 '14 at 04:44
2

This answer assumes that you have a free column on Sheet1. For this answer I'll assume that empty column is B.

In B1 put:

=COUNTIFS(Sheet2!A:A,Sheet1!A1)

And then drag it down through all rows in Sheet1 that have entries in column A. For example, cell B1234 would have:

=COUNTIFS(Sheet2!A:A,Sheet1!A1234)

Then you just have to write a VBA code to copy the entires that come up with something other than a 0.

Dim lastrow As Long, j As Long
lastrow = Worksheets("Sheet1").UsedRange.Rows.Count + 1
j = 1
For i = 1 To lastrow
    If Worksheets("Sheet1").Cells(i, 2) <> 0 Then
        Worksheets("Sheet3").Cells(j, 1) = Worksheets("Sheet1").Cells(i, 1)
        j = j + 1
    End If
Next i

Should work MUCH faster than you code. Of course you'd want to include the prefix and suffix suggested by hnk.

And you could always write some VBA to put column "B" in there and then remove it when it's done.

BeaumontTaz
  • 273
  • 2
  • 8
2

The fastest solution based on my experience is to move the data out of Excel worksheets (not out of Excel). Copy all data into arrays and then compare the array values. Mr.Excel, Ozgrid and Stackoverflow have all the documentation on how to move Ranges into Arrays.

Search query: "excel vba copy range to array stackoverflow"

Result: Excel VBA Copying range values to an Array,

I've dealt with 20,000 rows in less than 2 seconds. Your results may vary based on the math/comparisons you perform. You can place the results in another array and insert that array into Excel worksheet (fastest) or update the worksheet directly from the comparison.

Community
  • 1
  • 1
Jim
  • 66
  • 4
1

I'm using simple hack. If my script has some cycles I call this routine each Xth cycle, that allways helps.

Call Sleep(1)

Sub Sleep(Sec)
Dim S as Object: Set S = CreateObject("WScript.Shell")
S.Run "cmd /c ping localhost -n " & Sec, 0, True   End Sub