Can someone tell me if the following code is optimized and correct for Excel VBA to Uppercase on cell focus?
Sometime it slows the Excel. Could there be a better version?
Private Sub Worksheet_Change(ByVal Target As Range)
Dim cell As Range
Application.EnableEvents = False
For Each cell In Target
If IsEmpty(cell) Then
ElseIf cell.HasFormula = True Then
cell.Font.Name = "Arial Narrow"
cell.Font.Size = 9
Else
cell = UCase(cell)
cell.Font.Name = "Arial Narrow"
cell.Font.Size = 9
End If
Next
Application.EnableEvents = True
End Sub
The indentation is inconsistent, repeated statements can be reorganized so that no repetition is needed; empty code blocks shouldn't need to exist, a conditional expression that compares a Boolean
expression to a Boolean
literal is redundant, and implicit default member calls can be made explicit for clearer code:
Application.EnableEvents = False
Dim cell As Range
For Each cell In Target
If Not IsEmpty(cell.Value) Then
If Not cell.HasFormula Then
cell.Value = UCase(cell.Value)
End If
cell.Font.Name = "Arial Narrow"
cell.Font.Size = 9
End If
Next
Application.EnableEvents = True
Rubberduck (free, open-source; I manage the project) could help with indentation, and its code inspections would have warned about the empty block (and suggested inverting the conditional) and the implicit default member calls against Range
(cell.Value
).
As for performance, being a Worksheet.Change
event handler means the macro iterates every modified cell every time any cell is modified. Most of the time that would be for a single cell, so the loop only iterates once most of the time; you could probably earn a few milliseconds by disabling ScreenUpdating
and setting the Calculation
mode to xlCalculationManual
along with the EnableEvents
being set to True
, but IMO the benefits would be marginal.
Consider using styles instead of setting individual cells' Font.Name
and Font.Size
, but the most impactful modification to do here would be to test Target
for intersection with a particular specific interesting Range
of cells on the worksheet - for example if you wanted to only run that loop if the Target
is inside the A2:C25
range, you could have this test as the first thing the macro does, to prevent executing any code when the modified cell is outside that range:
If Application.Intersect(Target, Me.Range("A2:C25") Is Nothing Then
'modified cell isn't in A2:C25; bail out
Exit Sub
End If
Note that a Worksheet.Change
event handler procedure does not run "on cell focus", but "on cell value/formula modified"; look into handling Worksheet.SelectionChange
instead, to run code whenever a cell aquires Excel's selection rectangle.