Trying to make my VBA run a little bit faster-CodePudding

I am New to VBA, so my codes are usually very slow/suboptimized.

In one of my programs I have cells in the sheet that has to be filled when the user press a button, the renges change depending on the button but the concept is the same.

So I did this monstrosity:

Cells((Range("namedrange").Row   5), 1).Value = ThisWorkbook.Sheets(5).Cells(4, 7).Value
Cells((Range("namedrange").Row   5), 3).Value = ThisWorkbook.Sheets(5).Cells(4, 8).Value
Cells((Range("namedrange").Row   5), 5).Value = ThisWorkbook.Sheets(5).Cells(4, 9).Value
Cells((Range("namedrange").Row   5), 8).Value = ThisWorkbook.Sheets(5).Cells(4, 10).Value
Cells((Range("namedrange").Row   5)   1, 1).Value = ThisWorkbook.Sheets(5).Cells(5, 7).Value
Cells((Range("namedrange").Row   5)   1, 3).Value = ThisWorkbook.Sheets(5).Cells(5, 8).Value
Cells((Range("namedrange").Row   5)   1, 5).Value = ThisWorkbook.Sheets(5).Cells(5, 9).Value
Cells((Range("namedrange").Row   5)   1, 8).Value = ThisWorkbook.Sheets(5).Cells(5, 10).Value

but later changed to:

    With Range("namedrange")
        .Offset(5).Columns(1).Value = ThisWorkbook.Sheets(3).Cells(4, 7).Value
        .Offset(5).Columns(3).Value = ThisWorkbook.Sheets(3).Cells(4, 8).Value
        .Offset(5).Columns(5).Value = ThisWorkbook.Sheets(3).Cells(4, 9).Value
        .Offset(5).Columns(8).Value = ThisWorkbook.Sheets(3).Cells(4, 10).Value
        .Offset(6).Columns(1).Value = ThisWorkbook.Sheets(3).Cells(5, 7).Value
        .Offset(6).Columns(3).Value = ThisWorkbook.Sheets(3).Cells(5, 8).Value
        .Offset(6).Columns(5).Value = ThisWorkbook.Sheets(3).Cells(5, 9).Value
        .Offset(6).Columns(8).Value = ThisWorkbook.Sheets(3).Cells(5, 10).Value
    End With

which is a bit faster, however I feel that it is still suboptimized. And I would like to know if there is a way to make it cleaner/more elegant. Just to be noted that there are discontinuities in the columns, e.g. it starts in the 1st columns but jumps to the 3rd and then to the 5th and at last to the 8th.

The code works but it is slow, I just want a way to make it faster/cleaner.

CodePudding user response：

Using Variables

In regards to efficiency, that's about it: you're using the most efficient way to copy values from one cell to another aka copying by assignment.
If you want it to be more flexible, maintainable, and readable(?), here are some ideas.
Additionally, you can move the remaining magic numbers and text to constants at the beginning of the code or even use the constants as arguments.

Sub CopyValues()
    
    Dim wb As Workbook: Set wb = ThisWorkbook ' workbook containing this code
    
    ' Specify the worksheet if you know it.
    'Dim dnrg As Range: Set dnrg = wb.Sheets("Sheet1").Range("NamedRange")
    ' Otherwise, make sure the workbook is active.
    If Not wb Is ActiveWorkbook Then wb.Activate
    Dim dnrg As Range: Set dnrg = Range("NamedRange")
    
    Dim drg As Range: Set drg = dnrg.Range("A1,C1,E1,H1").Offset(5)
    Dim cCount As Long: cCount = drg.Cells.Count
    
    ' If you know the tab name, use it instead of the index (3).
    Dim sws As Worksheet: Set sws = wb.Sheets(3)
    Dim srg As Range: Set srg = sws.Range("G4").Resize(, cCount)
    
    Dim r As Long, c As Long
        
    For r = 0 To 1
        For c = 1 To cCount
            drg.Offset(r).Cells(c).Value = srg.Offset(r).Cells(c).Value
        Next c
    Next r

End Sub

CodePudding user response：

You can try to disable screenupdating during execution.

Disable ScreenUpdating

To disable ScreenUpdating, At the beginning of your code put this line:

Application.ScreenUpdating = False

Enable ScreenUpdating

To re-enable ScreenUpdating, At the end of your code put this line:

Application.ScreenUpdating = True

CodePudding user response：

I wonder what would happen if you use all the .Offset parameters, row and column. Example:

With Range("namedrange")
    .Offset(5, 0).Value = ThisWorkbook.Sheets(3).Cells(4, 7).Value
    .Offset(5, 2).Value = ThisWorkbook.Sheets(3).Cells(4, 8).Value
    .Offset(5, 4).Value = ThisWorkbook.Sheets(3).Cells(4, 9).Value
    .Offset(5, 7).Value = ThisWorkbook.Sheets(3).Cells(4, 10).Value
    .Offset(6, 0).Value = ThisWorkbook.Sheets(3).Cells(5, 7).Value
    .Offset(6, 2).Value = ThisWorkbook.Sheets(3).Cells(5, 8).Value
    .Offset(6, 4).Value = ThisWorkbook.Sheets(3).Cells(5, 9).Value
    .Offset(6, 7).Value = ThisWorkbook.Sheets(3).Cells(5, 10).Value
End With

CodePudding user response：

Accessing Excel for values from VBA is a slow operation and this adds up when you make multiple requests. When you are essentially retrieving the same information on a repetitive basis there are two two methods can be used to reduce access times.

Replace a lookup with a calculated value
Use a with statement

Thus you code could be written as

Dim myCol as long 
myCol =Range("namedrange").Row   5

With ThisWorkook.Sheets(5)
    Cells(myCol, 1).Value = .Cells(4, 7).Value
    Cells(myCol, 3).Value = .Cells(4, 8).Value
    Cells(myCol, 5).Value = .Cells(4, 9).Value
    Cells(myCol, 8).Value = .Cells(4, 10).Value
    myCol=myCol 1 ' trivial example
    Cells(mycol, 1).Value = .Cells(5, 7).Value
    Cells(myCol, 3).Value = .Cells(5, 8).Value
    Cells(myCol, 5).Value = .Cells(5, 9).Value
    Cells(myCol, 8).Value = .Cells(5, 10).Value
End with

Please also install the free, opensource, and fantastic Rubberduck addin for VBA. The code inspections will help you write VBA that is much more correct.