Excel VBA Explanation/Optimisation Needed

I am a self taught VBA user. I was asked to look at the code from a tool which has stopped working due to a data overflow and "fix" it. I was told the code was 'optimised' so that it took only 2 hours to run instead of four. They did this by remove loops.

I didn't have an issue with my 64-bit Excel, but another user stepped through it for me and the below lines prompted the 'out of memory' error.

Calculations are set to manual and the screen is frozen at the beginning of the code.

In my limited experience I've had faster results doing a loop and avoiding setting formulas in the document. There are around 500,000 lines of data currently.

Would I be better off changing it to a loop? Or would that add hours onto the computation time? I'm happy to post the full code if anyone wants to see it, but it is not annotated and none of the variables are defined so it is a bit of a mess. I'm cleaning it up as I decipher the code.

Sheets("Main Tab").Range("O2:O" & LastRow).Formula = "=IF(AND(N2>IF(VLOOKUP(CONCATENATE(E2,C2,D2,CalculateWeek),Velocity!J$2:N$" & VelocityLastRow & ",1,TRUE)=CONCATENATE(E2,C2,D2,CalculateWeek), VLOOKUP(CONCATENATE(E2,C2,D2,CalculateWeek),Velocity!J$2:N$" & VelocityLastRow & ",4,TRUE), ""Missing""), N2<>""""),IF(VLOOKUP(CONCATENATE(E2,C2,D2,CalculateWeek),Velocity!J$2:N$" & VelocityLastRow & ",1,TRUE)=CONCATENATE(E2,C2,D2,CalculateWeek), VLOOKUP(CONCATENATE(E2,C2,D2,CalculateWeek),Velocity!J$2:N$" & VelocityLastRow & ",4,TRUE), ""Missing""),N2)"
Sheets("Main Tab").Range("I2:O" & LastRow).Value = Sheets("Main Tab").Range("I2:O" & LastRow).Value

Full code (Calculate_Click is what is causing the issue):

```
Sub Clear_Click()

DisableOptimize
UnfilterAll
Application.Calculation = xlCalculationManual
Application.EnableEvents = False

MainTabLastColum = "AU"

Sheets("Order Upload").AutoFilterMode = False
Sheets("Main Tab").AutoFilterMode = False
Sheets("Microstrategy Data").AutoFilterMode = False
Sheets("

Oh, please forgive me if my tone come across abrasive - I know you're just trying to maintain and learn, which is why I bring this all up! Don't feel bad, I'm self-taught too and if I ran into that thing, I could not have fixed it when I started learning.

First things first, variables. Please use them. Right now the whole thing seems overwhelming because it is.

dim orderSheet as Worksheet
set orderSheet = Sheets("Order Upload")
'etc for the rest
Sheets("Main Tab")
Sheets("Microstrategy Data")
Sheets("Velocity")
Sheets("Quantity Available")

OR

Worksheets have a CodeName property - View Properties window (F4) and the (Name) field (the one at the top) can be used as the worksheet name. This way you can avoid Sheets("mySheet") and instead just use mySheet and you won't even have to declare variables!

Formulas, do you need to print them on the sheet?

Sheets("Main Tab").Range("AC2:AC" & LastRow).Formula = "=IF(R2<0,""C"","""")"

Why not make the calculation and then print that to the sheet? Or do you need them?

Dim MyRange4 As Range
Dim MyRange5 As Range

Goodness, what are these? Ranges? Doing what? Variable names - give your variables meaningful names. For instance:

For j = 2 To 500 ' LastRow
    Set MyRange4 = Worksheets("Main Tab").Range("A2:A" & LastRow)
    Set MyRange5 = Worksheets("Main Tab").Range("AA2:AA" & LastRow)
        myval4 = Application.WorksheetFunction.SumIf(MyRange4, Range("A" & j).Value, MyRange5)
    Worksheets("Main Tab").Cells(j, 31).Value = myval4
Next

I can't imagine what's going on here without going back through the entire module and figuring out what each thing is. Wouldn't it be easier to follow with something like:

For index = 2 To lastrow
    Set quantities = MainTab.Range("quantities")
    Set prices = MainTab.Range("prices")
    cost = 1 'calculation
    CostSheet.Range("total") = cost
Next

Or better yet, arrays. But one step at a time. Try refactoring all the hard-coded ranges and sheets into variables. Either CodeNames and Named Ranges or a range variable describing what the range is.

Speed

Formulas

If I search the macro for the word "formula" I come up with 53 hits. That's 53 different times you've set a formula. And many of those formulas are for more than one cell in a range. Of course excel will hang when calculations are turned back on - imagine how many calculations that is. If you can use values instead of formulas, please do. If not - please tell us why.

Loops

I see seven loops

For i = 2 to lastRow
For j = 2 to lastRow
For Each ws in thisworkbook.sheets
For j = 2 to LastRowSKU
For j = 2 to LastRowSKU
For j = 2 to LastRowSKU
For j = 2 to LastRowSKU

See those last 4? Or even all 6? Why are you iterating over that four separate times? Why not do everything in the single loop?

You also have

If lastRow > 1 Then

Five times.. in a row! Seems to me you could pull a function out of there for refactoring.

Also, speaking of lastrow - There is a standard way to find lastRow and lastColumn. That post explains why.

Example

You pointed to

Sheets("Main Tab").Range("O2:O" & LastRow).Formula = "=IF(AND(N2>IF(VLOOKUP(CONCATENATE(E2,C2,D2,CalculateWeek),Velocity!J$2:N$" & VelocityLastRow & ",1,TRUE)=CONCATENATE(E2,C2,D2,CalculateWeek), VLOOKUP(CONCATENATE(E2,C2,D2,CalculateWeek),Velocity!J$2:N$" & VelocityLastRow & ",4,TRUE), ""Missing""), N2<>""""),IF(VLOOKUP(CONCATENATE(E2,C2,D2,CalculateWeek),Velocity!J$2:N$" & VelocityLastRow & ",1,TRUE)=CONCATENATE(E2,C2,D2,CalculateWeek), VLOOKUP(CONCATENATE(E2,C2,D2,CalculateWeek),Velocity!J$2:N$" & VelocityLastRow & ",4,TRUE), ""Missing""),N2)"

as a single formula, right?

Const LOOKUP_MATCH as String = “Missing”
Dim lookupString as string
lookupString = CONCATENATE(E2,C2,D2,CalculateWeek)
dim velocityLookupRange as Range
set velocityLookupRange = Velocity.Range(cells(2,10),cells(lastRow,13))
dim lookupCell as Range
Set lookupCell = Range(“N2”)
Dim returnColumn as Long
ReturnColumn = 4

The formula would now be

MainTab("O2:O" & LastRow).Formula = "=IF(AND(LOOKUPCELL>IF(VLOOKUP(LookupString),VelocityLookupRange,1,1)=LookupString), VLOOKUP(LookupString),VelocityLookupRange,returnColumn,1), LOOKUP_MATCH), LOOKUPCELL<>""""),IF(VLOOKUP(LookupString),VelocityLookupRange,1,1)=LookupString), VLOOKUP(LookupString),VelocityLookupRange,returnColumn,1), LOOKUP_MATCH),LOOKUPCELL)"

Still overwhelming! Let's get out Notepad++ to figure this thing out:

Sheets("Main Tab").Range("O2:O" & LastRow).Formula =

```
=IF(
AND(
N2>IF(VLOOKUP(CONCATENATE(E2,C2,D2,CalculateWeek),Velocity!J$2:N$" & VelocityLastRow & ",1,TRUE)=CONCATENATE(E2,C2,D2,CalculateWeek), VLOOKUP(CONCATENATE(E2,C2,D2,CalculateWeek),Velocity!J$2:N$" & VelocityLastRow & ",4,TRUE), ""Missing""),
N2<>""""),
THEN
IF(VLOOKUP(CONCATENATE(E2,C2,D2,CalculateWeek),Velocity!J$2:N$" & VelocityLastRow & ",1,TRUE)=CONCATENATE(E2,C2,D2,CalculateWeek), VLOOKUP

StackExchange Code Review Q#156228, answer score: 6