Code running more slowly than on other files / dates

Question

I ran the below code looped for 6.5 thousand cells of criteria which are looked up against the range contained on the "LISTS" tab refered to. This range is some 20 thousand rows.

I ran the code numerous times yesterday in a test file and it ran very quickly. Maybe 2 minutes: if that.

Today, after deciding I was happy with the code, I've PASTED it (caps there because I'm wondering if that has something to do with it) into my main project. Now when I run the code, it takes 2 hours plus!

I didn't change any of the code except for sheet names.

Does anyone know of any reason for this that I'm missing?

I'm new to VBA so I'm suspecting it's some rookie error somewhere!

Dim x As Long
x = WorksheetFunction.CountA(Columns(1))

'define string length for CELL loop
Dim char As Integer
char = Len(ActiveCell)

'define cell loop name
Dim counter As Integer

'Begin RANGE loop
For Each cell In Range("b1:b" & x)
    cell.Activate

    'Incorporate CELL loop
    For counter = 1 To char
        'Determine if numeric value present in cell = TRUE or FALSE
        If IsNumeric(Right(Mid(ActiveCell, 1, counter), 1)) = True Then
            ActiveCell.Offset(0, 1).Value = Right(ActiveCell.Offset(0, 0), Len(ActiveCell.Offset(0, 0)) - counter + 1)
            Exit For
        Else
            ActiveCell.Offset(0, 1).Value = ActiveCell.Offset(0, 0)
        End If
    Next
Next

most likely your the workbook has events and or many formula. Turn off events and calculations at the beginning of the code and turn them back on at the end. — Scott Craner, Mar 27 '18 at 13:50
Apologies! My description of what the code does doesn't match the code! The code IS correct, but what it does was mis-communicated. This code is just testing for numeric values within a cell and then pulling out values from a string starting at the first numeric value. Sorry for the confusion. — Try Hard 3, Mar 27 '18 at 13:51
turn `application.screenupdating` to false at the beginning of your code, and back to true at the end. — Luuklag, Mar 27 '18 at 13:53
Thanks Scott but it's still on the go slow. I've run with screen updating off, calculations off and now, as per your suggestion, events turned off. — Try Hard 3, Mar 27 '18 at 14:00

score 2 · Answer 1 · answered Mar 27 '18 at 13:54

Try the code below, explanations inside the code's comments:

Dim x As Long
Dim char As Long 'define string length for CELL loop
Dim counter As Long 'define cell loop name

x = WorksheetFunction.CountA(Columns(1))

Application.ScreenUpdating = False ' will make your code run faster
Application.EnableEvents = False


'Begin RANGE loop
For Each cell In Range("b1:b" & x)
    'cell.Activate ' <--- no need to Activate, realy slows down your code

    'Incorporate CELL loop
    For counter = 1 To char

        'Determine if numeric value present in cell = TRUE or FALSE
        If IsNumeric(Right(Mid(cell.Value, 1, counter), 1)) = True Then
            cell.Offset(0, 1).Value = Right(cell.Value, Len(cell.Value) - counter + 1)
            Exit For
        Else
            cell.Offset(0, 1).Value = cell.Value
        End If
    Next counter
Next cell

Application.ScreenUpdating = True
Application.EnableEvents = True

@Try Hard 3 did you try the code above ? is it running fast enough ? — Shai Rado, Mar 27 '18 at 16:30
Radio - sorry I didn't realise you'd commented till now. No, the code is still running extremely slowly. When I first wrote it (even without your improvements) it ran very quickly. Now, even for a data set around 5% of the original size AND with your more efficient code, it's still running at a snail's pace (half an hour to an hour for 2000 lines and a max string character count of around 20) — Try Hard 3, Apr 04 '18 at 11:32

score 1 · Answer 2 · answered Mar 27 '18 at 13:52

1

You need to avoid the ActiveCell, as far as it slows your code. You are looping with for-each thus you can use the variable in the loop like this:

For Each cell In Range("b1:b" & x)
    For counter = 1 To char
        If IsNumeric(Right(Mid(cell, 1, counter), 1)) = True Then
            cell.Offset(0, 1).Value = Right(cell, Len(cell) - counter + 1)
            Exit For

        Else
            cell.Offset(0, 1) = cell.Offset(0, 0)

        End If
    Next
Next

Furthermore, things like cell.Offset(0, 0) are a bit useless. If you do not need Offset, do not write it. And in general:

answered Mar 27 '18 at 13:52

Vityata

42,633
8
55
100

& Shai Radio:- thanks for this very useful advice. It certainly sped things up and there are a few lessons in there for me going forward. HOWEVER: being true to the thread, I'm not sure I can mark either as an answer because I'm still not sure why the clumsy code I wrote ran in under 2 minutes yesterday but takes over an hour today...? I should probably clarify also that the file I used to test the code on yesterday is a duplicate of the main project file, (mentioning this to point out that any formulas, objects, etc. are the same in both files). – Try Hard 3 Mar 27 '18 at 15:31
My last comment @Vityata too - tried to "@"'s and failed! :-D – Try Hard 3 Mar 27 '18 at 15:32

score 0 · Accepted Answer · answered Apr 04 '18 at 11:59

Thanks to everyone who took the time to post on this one. Turns out I'm an IDIOT!!!

The first time I ran the code, I dsiabled autocalculation, and all this time when I was re-running it, I'd commented it out.

I'm new to VBA but there's no excuse for that! Agh!

So, the fix (as suggested by others on the thread):

enter before main body of the macro:

Application.Calculation = xlCalculationManual

then after the macro, enter:

Application.Calculation = xlCalculationAutomatic

Code running more slowly than on other files / dates

3 Answers3