Reputation: 53
I have some VBA code written that runs quite slowly. I have a series of different loops in my code. I know that loops aren't always the most efficient way to manipulate data, so I think they are the problem. I need ideas for how to either alter the loop or eliminate it so I can speed up the run time of my code.
Below is the most active loop I have created. It's running through all of the cells on row D (starting in D2) and manipulating their values based off of entries in the cells in row 1. If I can get help on this loop I'll probably be able to use similar techniques to alter the other loops in my code. Any tips are appreciated.
'sub work week for date range
Range("D2").Select
Do Until IsEmpty(ActiveCell.Value)
If IsEmpty(ActiveCell.Offset(-1, 0)) = False Then
ActiveCell.Value = ActiveCell.Offset(-1, 0).Value & "-" & Right(ActiveCell.Value, 4)
Else: ActiveCell.Value = ActiveCell.Value & "-" & Right(ActiveCell.Offset(0, -1), 4)
End If
ActiveCell.Offset(0, 1).Select
Loop
Upvotes: 2
Views: 342
Reputation: 386
For fast execution, my first recommendation is to turn automatic calculation and screen-updating off too if it still takes long.
I agree that anything that involves selecting is going to be incredibly slow so you should use range objects instead.
Final code:
' Declarations
Dim CurrentCell, LeftCell, PreviousCell As Range
Dim Last4Chars As String
'Initialize
Set CurrentCell = ActiveSheet.Range("D2")
'Optimizations
Application.Calculation = xlCalculationManual
Application.ScreenUpdating = False
'Loop until Current Cell in Empty
Do Until IsEmpty(CurrentCell.Value)
Set AboveCell = CurrentCell.Offset(-1, 0) 'One row above
Set LeftCell = CurrentCell.Offset(0, -1) 'One column left
If IsEmpty(AboveCell) = False Then
CurrentCell.Value = AboveCell.Value & "-" & Right(CurrentCell.Value, 4)
Else
CurrentCell.Value = CurrentCell.Value & "-" & Right(LeftCell, 4)
End If
Set CurrentCell = CurrentCell.Offset(0, 1)
Loop
'Optimizations reversed for normal use
Application.Calculation = xlCalculationAutomatic
Application.ScreenUpdating = True
Upvotes: 0
Reputation: 17647
The fastest, and more efficient method, would be as has been suggested in the comments by using arrays.
To get you to that point though, I've given you the first steps to improving your interaction with VBA and understanding how to write your code without selecting or activating objects:
For i = 4 To Cells(2, Columns.Count).End(xlToLeft).Column
With Cells(2, i)
If .Offset(-1, 0).Value = vbNullString Then
.Value = .Value & "-" & Right$(.Offset(0, -1).Value, 4)
Else
.Value = .Offset(-1, 0).Value & "-" & Right$(.Value, 4)
End If
End With
Next
Basically, you don't need to .Select
or .Activate
anything. Work with the objects directly and use a variable to dictate the column rather than activating the next cell.
Once you're comfortable with writing code in this style, look at assigning a range's value to a 2D array and then loop through the array instead.
Upvotes: 1