I've tried the below function over a range of 6000 cells.

'======================================
Private Function TrimAll(myTrimArea As Range)
Dim cell
For Each cell In myTrimArea.Cells
cell.Value = Trim(cell.Value)
Next cell
End Function
'======================================


.....it's very slow.
What is wrong with my code ?


Any help greatly appreciated
J

Assuming you are calling this from VBA and not using it as a User Defined
Function in a cell, perhaps:

Private Function TrimAll(myTrimArea As Range)
dim v, cell
v = myTrimArea.Value
For Each cell In v
cell = Trim(cell)
Next cell
MyTrimArea.Value = v
End Function

--
Regards,
Tom Ogilvy


"WhytheQ" wrote:

I've tried the below function over a range of 6000 cells.

'======================================
Private Function TrimAll(myTrimArea As Range)
Dim cell
For Each cell In myTrimArea.Cells
cell.Value = Trim(cell.Value)
Next cell
End Function
'======================================


.....it's very slow.
What is wrong with my code ?


Any help greatly appreciated
J

I'm wondering why you made this a function instead of a sub. How are you
calling the function?

"WhytheQ" wrote in message
oups.com...
I've tried the below function over a range of 6000 cells.

'======================================
Private Function TrimAll(myTrimArea As Range)
Dim cell
For Each cell In myTrimArea.Cells
cell.Value = Trim(cell.Value)
Next cell
End Function
'======================================


....it's very slow.
What is wrong with my code ?


Any help greatly appreciated
J

Whether is sub or function, this should help

'======================================
Private Function TrimAll(myTrimArea As Range)
Dim cell
application.screenupdating = false
For Each cell In myTrimArea.Cells
cell.Value = Trim(cell.Value)
Next cell
application.screenupdating = true
End Function

hth

Keith

nice one Keith
- screenupdating was turned off in the main routine which calls the
function, but not in the function itself.

J

On 24 Aug, 14:30, Keith74 wrote:
Whether is sub or function, this should help

'======================================
Private Function TrimAll(myTrimArea As Range)
Dim cell

application.screenupdating = false For Each cell In myTrimArea.Cells
cell.Value = Trim(cell.Value)
Next cell

application.screenupdating = true

End Function

hth

Keith

That would mean screenupdating was already turned off for your function as
shown by this modification of the help example for ScreenUpdating:

Sub ABC()
Dim ElapsedTime(2)
Application.ScreenUpdating = True
For i = 1 To 2
If i = 2 Then Application.ScreenUpdating = False
EFG ElapsedTime, i
Next i
Application.ScreenUpdating = True
MsgBox "Elapsed time, screen updating on: " & ElapsedTime(1) & _
" sec." & Chr(13) & _
"Elapsed time, screen updating off: " & ElapsedTime(2) & _
" sec."

End Sub

Sub EFG(ElapsedTime, i)
startTime = Time
Worksheets("Sheet1").Activate
For Each c In ActiveSheet.Columns
If c.Column Mod 2 = 0 Then
c.Hidden = True
End If
Next c
stopTime = Time
ElapsedTime(i) = (stopTime - startTime) * 24 * 60 * 60
End Sub

--
Regards,
Tom Ogilvy


"WhytheQ" wrote:

nice one Keith
- screenupdating was turned off in the main routine which calls the
function, but not in the function itself.

J

On 24 Aug, 14:30, Keith74 wrote:
Whether is sub or function, this should help

'======================================
Private Function TrimAll(myTrimArea As Range)
Dim cell

application.screenupdating = false For Each cell In myTrimArea.Cells
cell.Value = Trim(cell.Value)
Next cell

application.screenupdating = true

End Function

hth

Keith

On 24 Aug, 16:06, Tom Ogilvy
wrote:
That would mean screenupdating was already turned off for your function as
shown by this modification of the help example for ScreenUpdating:

Sub ABC()
Dim ElapsedTime(2)
Application.ScreenUpdating = True
For i = 1 To 2
If i = 2 Then Application.ScreenUpdating = False
EFG ElapsedTime, i
Next i
Application.ScreenUpdating = True
MsgBox "Elapsed time, screen updating on: " & ElapsedTime(1) & _
" sec." & Chr(13) & _
"Elapsed time, screen updating off: " & ElapsedTime(2) & _
" sec."

End Sub

Sub EFG(ElapsedTime, i)
startTime = Time
Worksheets("Sheet1").Activate
For Each c In ActiveSheet.Columns
If c.Column Mod 2 = 0 Then
c.Hidden = True
End If
Next c
stopTime = Time
ElapsedTime(i) = (stopTime - startTime) * 24 * 60 * 60
End Sub

--
Regards,
Tom Ogilvy



"WhytheQ" wrote:
nice one Keith
- screenupdating was turned off in the main routine which calls the
function, but not in the function itself.

J

On 24 Aug, 14:30, Keith74 wrote:
Whether is sub or function, this should help

'======================================
Private Function TrimAll(myTrimArea As Range)
Dim cell

application.screenupdating = false For Each cell In myTrimArea.Cells
cell.Value = Trim(cell.Value)
Next cell

application.screenupdating = true

End Function

hth

Keith- Hide quoted text -

- Show quoted text -



Thanks for the help Tom.
J


Unfortunately even with screenupdating on calling that function 18,000
times is still really slow, so I changed my tack to the below (column
Z was an empty column):
'======================================
Sub TrimAll(myColumn As Integer)
With Sheets("Data")
.Range("Z2").FormulaR1C1 = "=TRIM(RC" & myColumn & ")"
.Range("Z2").AutoFill .Range(.Cells(2, 26), .Cells(x, 26))
.Range(.Cells(2, 26), .Cells(x, 26)).Copy
.Range(.Cells(2, myColumn), .Cells(x,
myColumn)).PasteSpecial xlPasteValues
End With
End Sub
'======================================