Below Code not working on large selection-CodePudding

for large data selection (1000 cells) below query is not working but for small it's working. Excel stopped working unless i press escape.

Sub TrimReplaceAndUppercase()
    For Each cell In Selection
        If Not cell.HasFormula Then
            cell.Value = UCase(cell.Value)
            cell = Trim(cell)
            Selection.Replace What:="-", Replacement:="", LookAt:=xlPart, _
                SearchOrder:=xlByRows, MatchCase:=False, SearchFormat:=False, _
                ReplaceFormat:=False
        End If
    Next cell
End Sub

CodePudding user response：

Excel did not stop working. If Excel freezes it means Excel is still working! Because of a huge selection it needs much longer, so it looks like it is doing nothing.

I recommend to use VBA's replace() instead of the Range.Replace() and do it all in one step otherwise you have 3 read/write actions which makes it 3 times slower. Also turn off screen updating and calculation, to make it run faster.

Option Explicit

Public Sub TrimReplaceAndUppercase()
    Application.Calculation = xlCalculationManual
    Application.ScreenUpdating = False
    
    On Error GoTo CLEAN_EXIT ' in case anything goes wrong make sure to reactivate calculation and screenupdating

    Dim Cell As Range
    For Each Cell In Selection.Cells
        If Not Cell.HasFormula Then
            Cell.Value = Replace$(Trim(UCase(Cell.Value)), "-", vbNullString)
        End If
    Next Cell

CLEAN_EXIT:
    Application.ScreenUpdating = True
    Application.Calculation = xlCalculationAutomatic

    ' If there was an error above we want to know
    If Err.Number Then Err.Raise Err.Number
End Sub

The issue is this code

cell.Value = UCase(cell.Value)
cell = Trim(cell)
Selection.Replace What:="-", Replacement:="", LookAt:=xlPart, _
    SearchOrder:=xlByRows, MatchCase:=False, SearchFormat:=False, _
    ReplaceFormat:=False

means 3 times reading the cells content and 3 times writing it. Each read/write action takes a lot of time. And each write action triggers a calculation which takes time.

So first you want to minimize your read write actions to only 1 action. Reading data once from the cell, doing all the processing and writing it back once.

Second you don't want a calculation on each write action, so we set it to manual and in the end back to automatic. This will do only one calculation in the very end (for all changed cells) instead of a calculation for each single cell.

CodePudding user response：

Avoid the use of Selection. You may want to see How to avoid using Select in Excel VBA

If you want to still use Selection, then ensure it is a valid selection to minimize the error.

Here is another thing that you can try (Untested)

I have commented the code so you should not have a problem understanding it.

Option Explicit

Sub Sample()
    Dim rng As Range
    Dim aCell As Range
    
    On Error GoTo Whoa

    Application.ScreenUpdating = False
    Application.Calculation = xlCalculationManual
    
    '~~> Check if what the user selected is a valid range
    If TypeName(Selection) <> "Range" Then
        MsgBox "Select a range first."
        Exit Sub
    End If
    
    '~~> Work with only those cells which do not have formula
    On Error Resume Next
    Set rng = Selection.SpecialCells(xlCellTypeConstants)
    On Error GoTo 0
    
    '~~> Check if there are cells which have text
    If rng Is Nothing Then
        MsgBox "No cells with data found"
        Exit Sub
    End If
    
    '~~> Replace
    rng.Replace What:="-", Replacement:="", LookAt:=xlPart
    
    '~~> Trim and Uppercase in one line
    For Each aCell In rng
        aCell.Value = Trim(UCase(aCell.Value))
    Next aCell
    
LetsContinue:
    Application.ScreenUpdating = True
    Application.Calculation = xlCalculationAutomatic
    Exit Sub
Whoa:
    MsgBox Err.Description
    Resume LetsContinue
End Sub