Search code examples
excelvbams-officeuppercase

Excel VBA to Uppercase


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

Solution

  • 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.