Faster way to run little VBA macro?

Question

I am using this little code which takes nearly 250ms to run: (i is the increment of a loop on lines)

Sheets("sheet1").Select
val1 = Sheets("sheet2").Range("D" & Sheets("sheet1").Range("A" & i).Value + 1)
val2 = Sheets("sheet2").Range("E" & Sheets("sheet1").Range("A" & i).Value + 1)
Sheets("sheet1").Range("G" & i).Value = val1
Sheets("sheet1").Range("O" & i).Value = val2

I am not sure why this is taking so long for now. Is it because I move data from one sheet to another? Maybe using a variant will be faster?

Do you have any pointers on what could I do to make it faster? (this loop is done 300-400 times so the total time is too long.)

(not sure if this has a better place here or in "code reviews", let me know if I am wrong)

Thank you for your help!

[EDIT] Here is the loop after your suggested changes, thank you all, that reduced the time spent by 30%. However it is still too long for me, I have noted your suggestion to copy all the sheets sh1 and sh2in a table, I will try to implement that. If you have other ideas, please keep me posted =)

i = 2
While sh1.Cells(i, 1).Value <> ""
    val5 = sh1.Cells(i, 1).Value2 + 1
    With sh2
        val3 = .Cells(val5, 4).Value2
        val4 = .Cells(val5, 5).Value2
    End With

    With sh1
        .Cells(i, 7).Value2 = val3
        .Cells(i, 15).Value2 = val4
    End With

    i = i + 1
Wend

You will want to lessen the number of calls to the worksheet by loading everything into memory arrays. — Scott Craner, Oct 27 '16 at 14:13
Why store the values into variables only to use those variables on the next line? You could set `Sheets("sheet1").Range("G" & i).Value = Sheets("sheet2").Range("D" & Sheets("sheet1").Range("A" & i).Value + 1)` --- but combine that with other suggestions like arrays in memory — elmer007, Oct 27 '16 at 14:15
Can you show the loop code? There are ways to make it much faster by "copying" few cells at a time instead of one by one. — Slai, Oct 27 '16 at 16:13

score 3 · Answer 1 · answered Oct 27 '16 at 14:09

Don't use .Select (you don't seem to do anything with Selection afterwards anyway)
Store the sheets in two Variables: dim oSheet1 as Worksheet; set oSheet = Sheets("Sheet1"). Or use the code names: Sheet1 and Sheet2 (most likely)
It might be faster to use .Cells() than .Range() to avoid the string concatenation
If anything, avoid Variants as much as possible and declare type safe variables

score 3 · Accepted Answer · edited May 23 '17 at 11:48

3

Your code can modify as:

Dim sh as Worksheet, sh2 as Worksheet
Dim val1, val2
Set sh = Sheets("Sheet1")
Set sh2 = Sheets("Sheet2")
With sh
    val1 = sh2.Cells(.Cells(1, i).Value2 + 1, 4)
    val2 = sh2.Cells(.Cells(1, i).Value2 + 1, 5)
    .Cells(i, 7).Value = val1
    .Cells(i, 15).Value = val2
End With

And further reduced (unless you need to use val1 and val2 elsewhere:

Dim sh as Worksheet, sh2 as Worksheet, r as Long
Set sh = Sheets("Sheet1")
Set sh2 = Sheets("Sheet2")

With sh
    r = .Cells(1, i).Value2 + 1
    .Cells(i, 7).Value = sh2.Cells(r, 4).Value2
    .Cells(i, 15).Value = sh2.Cells(r, 5).Value2
End With

Additional information on how to avoid using select/activate, and proper use of variables can be found here:

How to avoid using Select in Excel VBA macros

edited May 23 '17 at 11:48

Community

1
1

answered Oct 27 '16 at 14:14

David Zemens

53,033
11
81
130

It just went down from 250ms to 170ms, thanks! Is there a difference between `Value` and `Value2`? – Bitoubi Oct 27 '16 at 14:32
1

http://stackoverflow.com/questions/17359835/what-is-the-difference-between-text-value-and-value2 – David Zemens Oct 27 '16 at 14:33
on that note, in the second code snippet above, use `sh2.Cells(r, 4).Value2` and `sh2.Cells(r, 5).Value2` and that might improve a little more. – David Zemens Oct 27 '16 at 14:34

score 1 · Answer 3 · answered Oct 27 '16 at 14:19

Get rid of the Sheets("sheet1").Select - it isn't doing anything in the code that you posted, and if this is in a loop it will repeatedly do nothing.

Get rid of the intermediary variables var1 and var2 and assign the values directly between the cells.

Use With blocks and stored object references to avoid repeated dereferencing of objects. You are performing repeated object lookups in the Excel Sheets collection.

Use .Cells with numeric indexing instead of Range with string concatenation and alpha references - Excel just has to convert them back anyway.

Cache intermediate values that are going to be used repeatedly (i.e. Sheets("sheet1").Range("A" & i).Value + 1).

Something more like:

'Outside loop:
Dim source As Worksheet
Dim target As Worksheet
Set source = Sheets("sheet2")
Set target = Sheets("sheet1")

'...
'Inside loop:
Dim targetRow As Long
With target
    targetRow = .Cells(i, 1).Value + 1
    .Cells(i, 7).Value = source.Cells(targetRow, 4)
    .Cells(i, 15).Value = source.Cells(targetRow, 5)
End With

If this still isn't fast enough, consider pulling the entire working Range into a Variant array and working with that instead. When you're done, write the entire thing back in one shot.

score 0 · Answer 4 · answered Nov 03 '16 at 14:31

As @ScottCraner commented, minimizing the calls to the application is one of the keys to faster VBA macros. For example, copying the whole column at once instead of cell by cell:

Dim c As Long
c = Sheet2.Cells(2, 1).End(xlDown).Row - 1  ' count = last row - first row - 1
Application.ScreenUpdating = False          ' optional
Sheet1.Cells(2,  7).Resize(c).Value2 = Sheet2.Cells(2, 4).Resize(c).Value2
Sheet1.Cells(2, 15).Resize(c).Value2 = Sheet2.Cells(2, 5).Resize(c).Value2
Application.ScreenUpdating = True           ' optional

Faster way to run little VBA macro?

4 Answers4