Trim Macro problem

[SimonHughes]

I have a TRIM macro that fails if I apply it to many cells. I almost always use it to trim cells in a single column. It is fine when working on a limited number, say 60 but more than that and I get an error message.

Sub ATrim()
Dim R As Long, C As Long, vRng As Variant
Application.ScreenUpdating = False
Application.EnableEvents = False
Application.Calculation = xlCalculationManual
vRng = Selection
For R = 1 To UBound(vRng, 1)
For C = 1 To UBound(vRng, 2)
vRng(R, C) = Trim(vRng(R, C))
Next
Next
Selection = vRng
Application.Calculation = xlCalculationAutomatic
Application.EnableEvents = True
Application.ScreenUpdating = True
End Sub

I am using Excel 2010 on Win 7

 

[VoG]

Try

Sub ATrim()
Dim c As Range
Application.ScreenUpdating = False
Application.EnableEvents = False
Application.Calculation = xlCalculationManual
For Each c In Selection
    c.Value = Trim(c.Value)
Next c
Application.Calculation = xlCalculationAutomatic
Application.EnableEvents = True
Application.ScreenUpdating = True
End Sub

 

[SimonHughes]

Hi there, it is still giving me a runtime error 13, mismatch.

 

[SimonHughes]

This is curious, when I use a helper column to TRIM, I get this - all the TRIM Size are the top one in the Size column?? There is no formula in the Size column.

<TABLE style="WIDTH: 178pt; BORDER-COLLAPSE: collapse" border=0 cellSpacing=0 cellPadding=0 width=238><COLGROUP><COL style="WIDTH: 89pt; mso-width-source: userset; mso-width-alt: 4352" span=2 width=119><TBODY><TR style="HEIGHT: 24pt" height=32><TD style="BORDER-BOTTOM: black 0.5pt solid; BORDER-LEFT: black 0.5pt solid; BACKGROUND-COLOR: gray; WIDTH: 89pt; HEIGHT: 24pt; BORDER-TOP: black 0.5pt solid; BORDER-RIGHT: black 0.5pt solid" class=xl63 height=32 width=119>Size</TD><TD style="BORDER-BOTTOM: black 0.5pt solid; BORDER-LEFT: black; BACKGROUND-COLOR: gray; WIDTH: 89pt; BORDER-TOP: black 0.5pt solid; BORDER-RIGHT: black 0.5pt solid" class=xl63 width=119>Trim Size</TD></TR><TR style="HEIGHT: 12.75pt" height=17><TD style="BORDER-BOTTOM: black 0.5pt solid; BORDER-LEFT: black 0.5pt solid; BACKGROUND-COLOR: transparent; WIDTH: 89pt; HEIGHT: 12.75pt; BORDER-TOP: black; BORDER-RIGHT: black 0.5pt solid" class=xl64 height=17 width=119>496X395X22</TD><TD style="BORDER-BOTTOM: black 0.5pt solid; BORDER-LEFT: black; BACKGROUND-COLOR: transparent; WIDTH: 89pt; BORDER-TOP: black; BORDER-RIGHT: black 0.5pt solid" class=xl64 width=119>496X395X22</TD></TR><TR style="HEIGHT: 12.75pt" height=17><TD style="BORDER-BOTTOM: black 0.5pt solid; BORDER-LEFT: black 0.5pt solid; BACKGROUND-COLOR: transparent; WIDTH: 89pt; HEIGHT: 12.75pt; BORDER-TOP: black; BORDER-RIGHT: black 0.5pt solid" class=xl64 height=17 width=119>496X395X47</TD><TD style="BORDER-BOTTOM: black 0.5pt solid; BORDER-LEFT: black; BACKGROUND-COLOR: transparent; WIDTH: 89pt; BORDER-TOP: black; BORDER-RIGHT: black 0.5pt solid" class=xl64 width=119>496X395X22</TD></TR><TR style="HEIGHT: 12.75pt" height=17><TD style="BORDER-BOTTOM: black 0.5pt solid; BORDER-LEFT: black 0.5pt solid; BACKGROUND-COLOR: transparent; WIDTH: 89pt; HEIGHT: 12.75pt; BORDER-TOP: black; BORDER-RIGHT: black 0.5pt solid" class=xl64 height=17 width=119>496X395X98</TD><TD style="BORDER-BOTTOM: black 0.5pt solid; BORDER-LEFT: black; BACKGROUND-COLOR: transparent; WIDTH: 89pt; BORDER-TOP: black; BORDER-RIGHT: black 0.5pt solid" class=xl64 width=119>496X395X22</TD></TR><TR style="HEIGHT: 12.75pt" height=17><TD style="BORDER-BOTTOM: black 0.5pt solid; BORDER-LEFT: black 0.5pt solid; BACKGROUND-COLOR: transparent; WIDTH: 89pt; HEIGHT: 12.75pt; BORDER-TOP: black; BORDER-RIGHT: black 0.5pt solid" class=xl64 height=17 width=119>496X496X22</TD><TD style="BORDER-BOTTOM: black 0.5pt solid; BORDER-LEFT: black; BACKGROUND-COLOR: transparent; WIDTH: 89pt; BORDER-TOP: black; BORDER-RIGHT: black 0.5pt solid" class=xl64 width=119>496X395X22</TD></TR><TR style="HEIGHT: 12.75pt" height=17><TD style="BORDER-BOTTOM: black 0.5pt solid; BORDER-LEFT: black 0.5pt solid; BACKGROUND-COLOR: transparent; WIDTH: 89pt; HEIGHT: 12.75pt; BORDER-TOP: black; BORDER-RIGHT: black 0.5pt solid" class=xl64 height=17 width=119>496X496X47</TD><TD style="BORDER-BOTTOM: black 0.5pt solid; BORDER-LEFT: black; BACKGROUND-COLOR: transparent; WIDTH: 89pt; BORDER-TOP: black; BORDER-RIGHT: black 0.5pt solid" class=xl64 width=119>496X395X22</TD></TR><TR style="HEIGHT: 12.75pt" height=17><TD style="BORDER-BOTTOM: black 0.5pt solid; BORDER-LEFT: black 0.5pt solid; BACKGROUND-COLOR: transparent; WIDTH: 89pt; HEIGHT: 12.75pt; BORDER-TOP: black; BORDER-RIGHT: black 0.5pt solid" class=xl64 height=17 width=119>496X496X47</TD><TD style="BORDER-BOTTOM: black 0.5pt solid; BORDER-LEFT: black; BACKGROUND-COLOR: transparent; WIDTH: 89pt; BORDER-TOP: black; BORDER-RIGHT: black 0.5pt solid" class=xl64 width=119>496X395X22</TD></TR></TBODY></TABLE>

 

[VoG]

Perhaps you have error values. Try

Sub ATrim()
Dim c As Range
Application.ScreenUpdating = False
Application.EnableEvents = False
Application.Calculation = xlCalculationManual
For Each c In Selection
    If Not IsError(c) Then
        c.Value = Trim(c.Value)
    End If
Next c
Application.Calculation = xlCalculationAutomatic
Application.EnableEvents = True
Application.ScreenUpdating = True
End Sub

 

[SimonHughes]

Excellent Peter, many thanks, now working a treat.

 

[Rick Rothstein]

Excellent Peter, many thanks, now working a treat.

Do you ever call your ATrim macro from within other code you write (you know, so that other code can trim some cells before it operates on them)? If so, then you should change your ATrim macro (the one Peter posted for you) to this...

Sub ATrim()
  Dim CurrentScreenUpdating As Long, CurrentCalculate As Long, CurrentEnableEvents As Long
  With Application
    CurrentScreenUpdating = .ScreenUpdating
    CurrentCalculate = .Calculation
    CurrentEnableEvents = .EnableEvents
    .ScreenUpdating = False
    .Calculation = xlCalculationManual
    .EnableEvents = False
  End With
  For Each c In Selection
      If Not IsError(c) Then
          c.Value = Trim(c.Value)
      End If
  Next c
  With Application
    .ScreenUpdating = CurrentScreenUpdating
    .Calculation = CurrentCalculate
    .EnableEvents = CurrentEnableEvents
  End With
End Sub

Note that I used With/End With blocks to reduce how many times the Application object was specified. The reason you should consider doing it this way is calling your ATrim subroutine from within other code that turns off ScreenUpdating, Calculation and/or EnableEvents will have that action undone when your ATrim code finishes running because it turns them all "back on". In general, any code subroutine or function that may be called from within other code should make sure it leaves those items the way it "found them" them using a structure like this...

<< Sub or Function >> SubFunctionName(<< Arguments if any >>)
 
  Dim CurrentScreenUpdating As Long, CurrentCalculate As Long, CurrentEnableEvents As Long
  << Your existing DIM statements go here >>
 
  With Application
    CurrentScreenUpdating = .ScreenUpdating
    CurrentCalculate = .Calculation
    CurrentEnableEvents = .EnableEvents
    .ScreenUpdating = False
    .Calculation = xlCalculationManual
    .EnableEvents = False
  End With
 
  << Your Sub or Function code goes here >>
 
  With Application
    .ScreenUpdating = CurrentScreenUpdating
    .Calculation = CurrentCalculate
    .EnableEvents = CurrentEnableEvents
  End With
 
End << Sub or Function >>

 

[SimonHughes]

Hi Rick, and thanks for the response. My problem is that I have a very (very) basic understanding of VBA and I can't even work out what your question is about subroutines etc. I will work on what you have sent me and see how it all goes (I will call them something else so I dont corrupt anything).

Much appreciated, thanks.

 

[Rick Rothstein]

Hi Rick, and thanks for the response. My problem is that I have a very (very) basic understanding of VBA and I can't even work out what your question is about subroutines etc. I will work on what you have sent me and see how it all goes (I will call them something else so I dont corrupt anything).

Subroutines (a macro is just a subroutine with no arguments) and/or functions can call other subroutines or functions from within themselves if they need that functionality. So your ATrim macro can be run by itself OR some other subroutine or function could call it in order to run it. When that happens, you sometimes have to be careful with what that first subroutine is doing as its actions can have consequences when called by other code. The caution I was giving you dealt with your turning the ScreenUpdating, Calculation and/or EnableEvents back on again at the end of your code. If I had a macro that I wanted to trim a range of cells before my code did things with those cells, I might choose to start my subroutine, call your ATrim subroutine and then proceed with my own code. The possible problem comes us this way... let's say to speed things up, I "turn off" ScreenUpdating, Calculation and EnableEvents as a first thing, then maybe do some code and then I call your subroutine... when your subroutine finishes, it turns ScreenUpdating, Calculation and/or EnableEvents all back on... but the rest of my code will execute after this... and it "thinks" those things are still off because they were turned off when my subroutine started. The code structure I last posted was meant to overcome that problem. Rather than turning those item off at the beginning and back on again at the end, the structure I posted memorizes the settings when it first runs and then puts those settings back on when finished... that way, any code that calls it will not be affected.

 

[SimonHughes]

Hi Rick, leaving the office for the day now but I shall get to grips with this tomorrow and give you some feedback. Many thanks