ExcelBanter

ExcelBanter (https://www.excelbanter.com/)
-   Excel Programming (https://www.excelbanter.com/excel-programming/)
-   -   Macro Takes Long Time To Run (https://www.excelbanter.com/excel-programming/425298-macro-takes-long-time-run.html)

Monk[_2_]

Macro Takes Long Time To Run
 
Hello

The macro below is taking an excessive amount of time to run. It takes about
2 or 3 minutes to complete. Can someone please review the code and see
whether there is a way to speed it up?

Thanks

Monk


Application.ScreenUpdating = False
Columns("A:o").Select
Selection.EntireColumn.Hidden = False
Rows("10:10").Select
With Selection.Font
.Name = "Arial"
.Size = 10
.Strikethrough = False
.Superscript = False
.Subscript = False
.OutlineFont = False
.Shadow = False
.Underline = xlUnderlineStyleNone
End With
Selection.RowHeight = 30#
Rows("11:1251").Select
With Selection.Font
.Name = "Arial"
.Size = 10
.Strikethrough = False
.Superscript = False
.Subscript = False
.OutlineFont = False
.Shadow = False
.Underline = xlUnderlineStyleNone
End With
Selection.RowHeight = 12.75
Dim c As Range
For Each c In Range("A11:A1250")
If c.Value = "" Then
Rows(c.Row).Hidden = True
Else
Rows(c.Row).Hidden = False
End If
Next c
Range("I2").Select
Application.ScreenUpdating = True
End Sub

Jon Peltier

Macro Takes Long Time To Run
 
I suspect the hiding of rows is your bottleneck, even though you use
Application.ScreenUpdating. Instead of hiding one row at a time, it is
faster to keep track of rows in a range, and hide the range all at once.

This is a shortened test version of your code:

Sub Test1()
Dim c As Range
Dim t As Double
Dim i As Long
t = Timer
Application.ScreenUpdating = False
For i = 1 To 100
ActiveSheet.Range("A1:A40").EntireColumn.Hidden = False
For Each c In ActiveSheet.Range("A1:A40").Cells
If c.Value = "" Then
c.EntireRow.Hidden = True
Else
c.EntireRow.Hidden = False
End If
Next
Next
Application.ScreenUpdating = True
Debug.Print Timer - t
End Sub

I ran this five times, with an average elapsed time of 5.8 seconds.

This does the same by adding each cell 'c' to a range 'r', then hiding this
multicell range:

Sub Test3()
Dim c As Range
Dim r As Range
Dim t As Double
Dim i As Long
t = Timer
Application.ScreenUpdating = False
For i = 1 To 100
ActiveSheet.Range("A1:A40").EntireColumn.Hidden = False
Set r = Nothing
For Each c In ActiveSheet.Range("A1:A40").Cells
If Len(c.Value) = 0 Then
If r Is Nothing Then
Set r = c
Else
Set r = Union(r, c)
End If
End If
Next
r.EntireRow.Hidden = True
Next
Application.ScreenUpdating = True
Debug.Print Timer - t
End Sub

Five iterations averaged 1.6 seconds.

- Jon
-------
Jon Peltier, Microsoft Excel MVP
Peltier Technical Services, Inc.
http://PeltierTech.com/WordPress/
_______


"Monk" wrote in message
...
Hello

The macro below is taking an excessive amount of time to run. It takes
about
2 or 3 minutes to complete. Can someone please review the code and see
whether there is a way to speed it up?

Thanks

Monk


Application.ScreenUpdating = False
Columns("A:o").Select
Selection.EntireColumn.Hidden = False
Rows("10:10").Select
With Selection.Font
.Name = "Arial"
.Size = 10
.Strikethrough = False
.Superscript = False
.Subscript = False
.OutlineFont = False
.Shadow = False
.Underline = xlUnderlineStyleNone
End With
Selection.RowHeight = 30#
Rows("11:1251").Select
With Selection.Font
.Name = "Arial"
.Size = 10
.Strikethrough = False
.Superscript = False
.Subscript = False
.OutlineFont = False
.Shadow = False
.Underline = xlUnderlineStyleNone
End With
Selection.RowHeight = 12.75
Dim c As Range
For Each c In Range("A11:A1250")
If c.Value = "" Then
Rows(c.Row).Hidden = True
Else
Rows(c.Row).Hidden = False
End If
Next c
Range("I2").Select
Application.ScreenUpdating = True
End Sub




Jim Cone[_2_]

Macro Takes Long Time To Run
 
This ran pretty fast too. However, it was run on xl2003 not on XL2007.
'--
Sub tract()
Dim c As Range
Application.ScreenUpdating = False
Application.Calculation = xlCalculationManual
Columns("A:O").EntireColumn.Hidden = False
With Rows(10).Font
.Name = "Arial"
.Size = 10
End With
Rows(10).RowHeight = 30
With Rows("11:1251").Font
.Name = "Arial"
.Size = 10
End With
Rows("11:1251").RowHeight = 12.75

For Each c In Range("A11:A1250").Cells
c.EntireRow.Hidden = IsEmpty(c)
Next c

Range("I2").Select
Application.Calculation = xlCalculationAutomatic
Application.ScreenUpdating = True
End Sub
--
Jim Cone
Portland, Oregon USA




"Monk"
wrote in message
Hello
The macro below is taking an excessive amount of time to run. It takes about
2 or 3 minutes to complete. Can someone please review the code and see
whether there is a way to speed it up?
Thanks
Monk

Application.ScreenUpdating = False
Columns("A:o").Select
Selection.EntireColumn.Hidden = False
Rows("10:10").Select
With Selection.Font
.Name = "Arial"
.Size = 10
.Strikethrough = False
.Superscript = False
.Subscript = False
.OutlineFont = False
.Shadow = False
.Underline = xlUnderlineStyleNone
End With
Selection.RowHeight = 30#
Rows("11:1251").Select
With Selection.Font
.Name = "Arial"
.Size = 10
.Strikethrough = False
.Superscript = False
.Subscript = False
.OutlineFont = False
.Shadow = False
.Underline = xlUnderlineStyleNone
End With
Selection.RowHeight = 12.75
Dim c As Range
For Each c In Range("A11:A1250")
If c.Value = "" Then
Rows(c.Row).Hidden = True
Else
Rows(c.Row).Hidden = False
End If
Next c
Range("I2").Select
Application.ScreenUpdating = True
End Sub

Monk[_2_]

Macro Takes Long Time To Run
 
Thanks Jim this works much quicker.

However the code to hide the rows if empty doesn't appear to work. I am
guessing this is because there is a "" value in the cell as a result of a
formula. Can you advise how this line should be amended to hide rows if there
is a "" value in the A column cell?

Cheers

"Jim Cone" wrote:

This ran pretty fast too. However, it was run on xl2003 not on XL2007.
'--
Sub tract()
Dim c As Range
Application.ScreenUpdating = False
Application.Calculation = xlCalculationManual
Columns("A:O").EntireColumn.Hidden = False
With Rows(10).Font
.Name = "Arial"
.Size = 10
End With
Rows(10).RowHeight = 30
With Rows("11:1251").Font
.Name = "Arial"
.Size = 10
End With
Rows("11:1251").RowHeight = 12.75

For Each c In Range("A11:A1250").Cells
c.EntireRow.Hidden = IsEmpty(c)
Next c

Range("I2").Select
Application.Calculation = xlCalculationAutomatic
Application.ScreenUpdating = True
End Sub
--
Jim Cone
Portland, Oregon USA




"Monk"
wrote in message
Hello
The macro below is taking an excessive amount of time to run. It takes about
2 or 3 minutes to complete. Can someone please review the code and see
whether there is a way to speed it up?
Thanks
Monk

Application.ScreenUpdating = False
Columns("A:o").Select
Selection.EntireColumn.Hidden = False
Rows("10:10").Select
With Selection.Font
.Name = "Arial"
.Size = 10
.Strikethrough = False
.Superscript = False
.Subscript = False
.OutlineFont = False
.Shadow = False
.Underline = xlUnderlineStyleNone
End With
Selection.RowHeight = 30#
Rows("11:1251").Select
With Selection.Font
.Name = "Arial"
.Size = 10
.Strikethrough = False
.Superscript = False
.Subscript = False
.OutlineFont = False
.Shadow = False
.Underline = xlUnderlineStyleNone
End With
Selection.RowHeight = 12.75
Dim c As Range
For Each c In Range("A11:A1250")
If c.Value = "" Then
Rows(c.Row).Hidden = True
Else
Rows(c.Row).Hidden = False
End If
Next c
Range("I2").Select
Application.ScreenUpdating = True
End Sub


Simon Lloyd[_1059_]

Macro Takes Long Time To Run
 

Jim it does run a little slower in 2007 than Jons' code however it was
marginal.

Jim Cone;262696 Wrote:
This ran pretty fast too. However, it was run on xl2003 not on XL2007.
'--
Sub tract()
Dim c As Range
Application.ScreenUpdating = False
Application.Calculation = xlCalculationManual
Columns("A:O").EntireColumn.Hidden = False
With Rows(10).Font
.Name = "Arial"
.Size = 10
End With
Rows(10).RowHeight = 30
With Rows("11:1251").Font
.Name = "Arial"
.Size = 10
End With
Rows("11:1251").RowHeight = 12.75

For Each c In Range("A11:A1250").Cells
c.EntireRow.Hidden = IsEmpty(c)
Next c

Range("I2").Select
Application.Calculation = xlCalculationAutomatic
Application.ScreenUpdating = True
End Sub
--
Jim Cone
Portland, Oregon USA




"Monk"
wrote in message
Hello
The macro below is taking an excessive amount of time to run. It takes
about
2 or 3 minutes to complete. Can someone please review the code and see
whether there is a way to speed it up?
Thanks
Monk

Application.ScreenUpdating = False
Columns("A:o").Select
Selection.EntireColumn.Hidden = False
Rows("10:10").Select
With Selection.Font
.Name = "Arial"
.Size = 10
.Strikethrough = False
.Superscript = False
.Subscript = False
.OutlineFont = False
.Shadow = False
.Underline = xlUnderlineStyleNone
End With
Selection.RowHeight = 30#
Rows("11:1251").Select
With Selection.Font
.Name = "Arial"
.Size = 10
.Strikethrough = False
.Superscript = False
.Subscript = False
.OutlineFont = False
.Shadow = False
.Underline = xlUnderlineStyleNone
End With
Selection.RowHeight = 12.75
Dim c As Range
For Each c In Range("A11:A1250")
If c.Value = "" Then
Rows(c.Row).Hidden = True
Else
Rows(c.Row).Hidden = False
End If
Next c
Range("I2").Select
Application.ScreenUpdating = True
End Sub



--
Simon Lloyd

Regards,
Simon Lloyd
'The Code Cage' (http://www.thecodecage.com)
------------------------------------------------------------------------
Simon Lloyd's Profile: http://www.thecodecage.com/forumz/member.php?userid=1
View this thread: http://www.thecodecage.com/forumz/sh...ad.php?t=73338


Monk[_2_]

Macro Takes Long Time To Run
 
Hi Jon

I have used your code as shown below. I must have made an error somewhere as
it still takes a couple of minutes to complete. Are you able to advise where
my error is?

Monk


Application.ScreenUpdating = False
Application.Calculation = xlCalculationManual
Columns("A:o").EntireColumn.Hidden = False
With Rows(10).Font
.Name = "Arial"
.Size = 10
End With
Rows(10).RowHeight = 30
With Rows("11:1251").Font
.Name = "Arial"
.Size = 10
End With
Rows("11:1251").RowHeight = 12.75

Dim c As Range
Dim t As Double
Dim i As Long
t = Timer
For i = 1 To 100
ActiveSheet.Range("A11:A1250").EntireColumn.Hidden = False
For Each c In ActiveSheet.Range("A11:A1250").Cells
If c.Value = "" Then
c.EntireRow.Hidden = True
Else
c.EntireRow.Hidden = False
End If
Next
Next
Debug.Print Timer - t
Range("I2").Select
Application.Calculation = xlCalculationAutomatic
Application.ScreenUpdating = True
End Sub

"Jon Peltier" wrote:

I suspect the hiding of rows is your bottleneck, even though you use
Application.ScreenUpdating. Instead of hiding one row at a time, it is
faster to keep track of rows in a range, and hide the range all at once.

This is a shortened test version of your code:

Sub Test1()
Dim c As Range
Dim t As Double
Dim i As Long
t = Timer
Application.ScreenUpdating = False
For i = 1 To 100
ActiveSheet.Range("A1:A40").EntireColumn.Hidden = False
For Each c In ActiveSheet.Range("A1:A40").Cells
If c.Value = "" Then
c.EntireRow.Hidden = True
Else
c.EntireRow.Hidden = False
End If
Next
Next
Application.ScreenUpdating = True
Debug.Print Timer - t
End Sub

I ran this five times, with an average elapsed time of 5.8 seconds.

This does the same by adding each cell 'c' to a range 'r', then hiding this
multicell range:

Sub Test3()
Dim c As Range
Dim r As Range
Dim t As Double
Dim i As Long
t = Timer
Application.ScreenUpdating = False
For i = 1 To 100
ActiveSheet.Range("A1:A40").EntireColumn.Hidden = False
Set r = Nothing
For Each c In ActiveSheet.Range("A1:A40").Cells
If Len(c.Value) = 0 Then
If r Is Nothing Then
Set r = c
Else
Set r = Union(r, c)
End If
End If
Next
r.EntireRow.Hidden = True
Next
Application.ScreenUpdating = True
Debug.Print Timer - t
End Sub

Five iterations averaged 1.6 seconds.

- Jon
-------
Jon Peltier, Microsoft Excel MVP
Peltier Technical Services, Inc.
http://PeltierTech.com/WordPress/
_______


"Monk" wrote in message
...
Hello

The macro below is taking an excessive amount of time to run. It takes
about
2 or 3 minutes to complete. Can someone please review the code and see
whether there is a way to speed it up?

Thanks

Monk


Application.ScreenUpdating = False
Columns("A:o").Select
Selection.EntireColumn.Hidden = False
Rows("10:10").Select
With Selection.Font
.Name = "Arial"
.Size = 10
.Strikethrough = False
.Superscript = False
.Subscript = False
.OutlineFont = False
.Shadow = False
.Underline = xlUnderlineStyleNone
End With
Selection.RowHeight = 30#
Rows("11:1251").Select
With Selection.Font
.Name = "Arial"
.Size = 10
.Strikethrough = False
.Superscript = False
.Subscript = False
.OutlineFont = False
.Shadow = False
.Underline = xlUnderlineStyleNone
End With
Selection.RowHeight = 12.75
Dim c As Range
For Each c In Range("A11:A1250")
If c.Value = "" Then
Rows(c.Row).Hidden = True
Else
Rows(c.Row).Hidden = False
End If
Next c
Range("I2").Select
Application.ScreenUpdating = True
End Sub





Jon Peltier

Macro Takes Long Time To Run
 
Monk -

I was showing a comparison between two techniques, a slow way and a fast
way. Each looped 100 times to give timer values that could be compared. You
took the slower code, and kept in the loop. So yeah, it's going to be slow.

What you need to do is replace this slow bit of your code:

Dim c As Range
For Each c In Range("A11:A1250")
If c.Value = "" Then
Rows(c.Row).Hidden = True
Else
Rows(c.Row).Hidden = False
End If
Next c

with this faster bit:

Dim c As Range
Dim r As Range
For Each c In Range("A11:A1250")
If c.Value = "" Then
If r is nothing then
Set r = c
else
Set r = Union(r. c)
endif
End If
Next c
r.EntireRow.Hidden = True

- Jon
-------
Jon Peltier, Microsoft Excel MVP
Peltier Technical Services, Inc.
http://PeltierTech.com/WordPress/
_______


"Monk" wrote in message
...
Hi Jon

I have used your code as shown below. I must have made an error somewhere
as
it still takes a couple of minutes to complete. Are you able to advise
where
my error is?

Monk


Application.ScreenUpdating = False
Application.Calculation = xlCalculationManual
Columns("A:o").EntireColumn.Hidden = False
With Rows(10).Font
.Name = "Arial"
.Size = 10
End With
Rows(10).RowHeight = 30
With Rows("11:1251").Font
.Name = "Arial"
.Size = 10
End With
Rows("11:1251").RowHeight = 12.75

Dim c As Range
Dim t As Double
Dim i As Long
t = Timer
For i = 1 To 100
ActiveSheet.Range("A11:A1250").EntireColumn.Hidden = False
For Each c In ActiveSheet.Range("A11:A1250").Cells
If c.Value = "" Then
c.EntireRow.Hidden = True
Else
c.EntireRow.Hidden = False
End If
Next
Next
Debug.Print Timer - t
Range("I2").Select
Application.Calculation = xlCalculationAutomatic
Application.ScreenUpdating = True
End Sub

"Jon Peltier" wrote:

I suspect the hiding of rows is your bottleneck, even though you use
Application.ScreenUpdating. Instead of hiding one row at a time, it is
faster to keep track of rows in a range, and hide the range all at once.

This is a shortened test version of your code:

Sub Test1()
Dim c As Range
Dim t As Double
Dim i As Long
t = Timer
Application.ScreenUpdating = False
For i = 1 To 100
ActiveSheet.Range("A1:A40").EntireColumn.Hidden = False
For Each c In ActiveSheet.Range("A1:A40").Cells
If c.Value = "" Then
c.EntireRow.Hidden = True
Else
c.EntireRow.Hidden = False
End If
Next
Next
Application.ScreenUpdating = True
Debug.Print Timer - t
End Sub

I ran this five times, with an average elapsed time of 5.8 seconds.

This does the same by adding each cell 'c' to a range 'r', then hiding
this
multicell range:

Sub Test3()
Dim c As Range
Dim r As Range
Dim t As Double
Dim i As Long
t = Timer
Application.ScreenUpdating = False
For i = 1 To 100
ActiveSheet.Range("A1:A40").EntireColumn.Hidden = False
Set r = Nothing
For Each c In ActiveSheet.Range("A1:A40").Cells
If Len(c.Value) = 0 Then
If r Is Nothing Then
Set r = c
Else
Set r = Union(r, c)
End If
End If
Next
r.EntireRow.Hidden = True
Next
Application.ScreenUpdating = True
Debug.Print Timer - t
End Sub

Five iterations averaged 1.6 seconds.

- Jon
-------
Jon Peltier, Microsoft Excel MVP
Peltier Technical Services, Inc.
http://PeltierTech.com/WordPress/
_______


"Monk" wrote in message
...
Hello

The macro below is taking an excessive amount of time to run. It takes
about
2 or 3 minutes to complete. Can someone please review the code and see
whether there is a way to speed it up?

Thanks

Monk


Application.ScreenUpdating = False
Columns("A:o").Select
Selection.EntireColumn.Hidden = False
Rows("10:10").Select
With Selection.Font
.Name = "Arial"
.Size = 10
.Strikethrough = False
.Superscript = False
.Subscript = False
.OutlineFont = False
.Shadow = False
.Underline = xlUnderlineStyleNone
End With
Selection.RowHeight = 30#
Rows("11:1251").Select
With Selection.Font
.Name = "Arial"
.Size = 10
.Strikethrough = False
.Superscript = False
.Subscript = False
.OutlineFont = False
.Shadow = False
.Underline = xlUnderlineStyleNone
End With
Selection.RowHeight = 12.75
Dim c As Range
For Each c In Range("A11:A1250")
If c.Value = "" Then
Rows(c.Row).Hidden = True
Else
Rows(c.Row).Hidden = False
End If
Next c
Range("I2").Select
Application.ScreenUpdating = True
End Sub







Jon Peltier

Macro Takes Long Time To Run
 
Change

c.EntireRow.Hidden = IsEmpty(c)

to

c.EntireRow.Hidden = (Len(c.Value) = 0)

- Jon
-------
Jon Peltier, Microsoft Excel MVP
Peltier Technical Services, Inc.
http://PeltierTech.com/WordPress/
_______


"Monk" wrote in message
...
Thanks Jim this works much quicker.

However the code to hide the rows if empty doesn't appear to work. I am
guessing this is because there is a "" value in the cell as a result of a
formula. Can you advise how this line should be amended to hide rows if
there
is a "" value in the A column cell?

Cheers

"Jim Cone" wrote:

This ran pretty fast too. However, it was run on xl2003 not on XL2007.
'--
Sub tract()
Dim c As Range
Application.ScreenUpdating = False
Application.Calculation = xlCalculationManual
Columns("A:O").EntireColumn.Hidden = False
With Rows(10).Font
.Name = "Arial"
.Size = 10
End With
Rows(10).RowHeight = 30
With Rows("11:1251").Font
.Name = "Arial"
.Size = 10
End With
Rows("11:1251").RowHeight = 12.75

For Each c In Range("A11:A1250").Cells
c.EntireRow.Hidden = IsEmpty(c)
Next c

Range("I2").Select
Application.Calculation = xlCalculationAutomatic
Application.ScreenUpdating = True
End Sub
--
Jim Cone
Portland, Oregon USA




"Monk"
wrote in message
Hello
The macro below is taking an excessive amount of time to run. It takes
about
2 or 3 minutes to complete. Can someone please review the code and see
whether there is a way to speed it up?
Thanks
Monk

Application.ScreenUpdating = False
Columns("A:o").Select
Selection.EntireColumn.Hidden = False
Rows("10:10").Select
With Selection.Font
.Name = "Arial"
.Size = 10
.Strikethrough = False
.Superscript = False
.Subscript = False
.OutlineFont = False
.Shadow = False
.Underline = xlUnderlineStyleNone
End With
Selection.RowHeight = 30#
Rows("11:1251").Select
With Selection.Font
.Name = "Arial"
.Size = 10
.Strikethrough = False
.Superscript = False
.Subscript = False
.OutlineFont = False
.Shadow = False
.Underline = xlUnderlineStyleNone
End With
Selection.RowHeight = 12.75
Dim c As Range
For Each c In Range("A11:A1250")
If c.Value = "" Then
Rows(c.Row).Hidden = True
Else
Rows(c.Row).Hidden = False
End If
Next c
Range("I2").Select
Application.ScreenUpdating = True
End Sub




Jon Peltier

Macro Takes Long Time To Run
 
I didn't try mine in 2007, but it will be slower there than in 2003. 2002
would probably be fastest of all.

- Jon
-------
Jon Peltier, Microsoft Excel MVP
Peltier Technical Services, Inc.
http://PeltierTech.com/WordPress/
_______


"Simon Lloyd" wrote in message
...

Jim it does run a little slower in 2007 than Jons' code however it was
marginal.

Jim Cone;262696 Wrote:
This ran pretty fast too. However, it was run on xl2003 not on XL2007.
'--
Sub tract()
Dim c As Range
Application.ScreenUpdating = False
Application.Calculation = xlCalculationManual
Columns("A:O").EntireColumn.Hidden = False
With Rows(10).Font
.Name = "Arial"
.Size = 10
End With
Rows(10).RowHeight = 30
With Rows("11:1251").Font
.Name = "Arial"
.Size = 10
End With
Rows("11:1251").RowHeight = 12.75

For Each c In Range("A11:A1250").Cells
c.EntireRow.Hidden = IsEmpty(c)
Next c

Range("I2").Select
Application.Calculation = xlCalculationAutomatic
Application.ScreenUpdating = True
End Sub
--
Jim Cone
Portland, Oregon USA




"Monk"
wrote in message
Hello
The macro below is taking an excessive amount of time to run. It takes
about
2 or 3 minutes to complete. Can someone please review the code and see
whether there is a way to speed it up?
Thanks
Monk

Application.ScreenUpdating = False
Columns("A:o").Select
Selection.EntireColumn.Hidden = False
Rows("10:10").Select
With Selection.Font
.Name = "Arial"
.Size = 10
.Strikethrough = False
.Superscript = False
.Subscript = False
.OutlineFont = False
.Shadow = False
.Underline = xlUnderlineStyleNone
End With
Selection.RowHeight = 30#
Rows("11:1251").Select
With Selection.Font
.Name = "Arial"
.Size = 10
.Strikethrough = False
.Superscript = False
.Subscript = False
.OutlineFont = False
.Shadow = False
.Underline = xlUnderlineStyleNone
End With
Selection.RowHeight = 12.75
Dim c As Range
For Each c In Range("A11:A1250")
If c.Value = "" Then
Rows(c.Row).Hidden = True
Else
Rows(c.Row).Hidden = False
End If
Next c
Range("I2").Select
Application.ScreenUpdating = True
End Sub



--
Simon Lloyd

Regards,
Simon Lloyd
'The Code Cage' (http://www.thecodecage.com)
------------------------------------------------------------------------
Simon Lloyd's Profile:
http://www.thecodecage.com/forumz/member.php?userid=1
View this thread: http://www.thecodecage.com/forumz/sh...ad.php?t=73338




Monk[_2_]

Macro Takes Long Time To Run
 
Thanks Jon. Sorry to be a pain but I am getting a "Compile Error: Argument
not optional" response on the Union (r,c) line

"Jon Peltier" wrote:

Monk -

I was showing a comparison between two techniques, a slow way and a fast
way. Each looped 100 times to give timer values that could be compared. You
took the slower code, and kept in the loop. So yeah, it's going to be slow.

What you need to do is replace this slow bit of your code:

Dim c As Range
For Each c In Range("A11:A1250")
If c.Value = "" Then
Rows(c.Row).Hidden = True
Else
Rows(c.Row).Hidden = False
End If
Next c

with this faster bit:

Dim c As Range
Dim r As Range
For Each c In Range("A11:A1250")
If c.Value = "" Then
If r is nothing then
Set r = c
else
Set r = Union(r. c)
endif
End If
Next c
r.EntireRow.Hidden = True

- Jon
-------
Jon Peltier, Microsoft Excel MVP
Peltier Technical Services, Inc.
http://PeltierTech.com/WordPress/
_______


"Monk" wrote in message
...
Hi Jon

I have used your code as shown below. I must have made an error somewhere
as
it still takes a couple of minutes to complete. Are you able to advise
where
my error is?

Monk


Application.ScreenUpdating = False
Application.Calculation = xlCalculationManual
Columns("A:o").EntireColumn.Hidden = False
With Rows(10).Font
.Name = "Arial"
.Size = 10
End With
Rows(10).RowHeight = 30
With Rows("11:1251").Font
.Name = "Arial"
.Size = 10
End With
Rows("11:1251").RowHeight = 12.75

Dim c As Range
Dim t As Double
Dim i As Long
t = Timer
For i = 1 To 100
ActiveSheet.Range("A11:A1250").EntireColumn.Hidden = False
For Each c In ActiveSheet.Range("A11:A1250").Cells
If c.Value = "" Then
c.EntireRow.Hidden = True
Else
c.EntireRow.Hidden = False
End If
Next
Next
Debug.Print Timer - t
Range("I2").Select
Application.Calculation = xlCalculationAutomatic
Application.ScreenUpdating = True
End Sub

"Jon Peltier" wrote:

I suspect the hiding of rows is your bottleneck, even though you use
Application.ScreenUpdating. Instead of hiding one row at a time, it is
faster to keep track of rows in a range, and hide the range all at once.

This is a shortened test version of your code:

Sub Test1()
Dim c As Range
Dim t As Double
Dim i As Long
t = Timer
Application.ScreenUpdating = False
For i = 1 To 100
ActiveSheet.Range("A1:A40").EntireColumn.Hidden = False
For Each c In ActiveSheet.Range("A1:A40").Cells
If c.Value = "" Then
c.EntireRow.Hidden = True
Else
c.EntireRow.Hidden = False
End If
Next
Next
Application.ScreenUpdating = True
Debug.Print Timer - t
End Sub

I ran this five times, with an average elapsed time of 5.8 seconds.

This does the same by adding each cell 'c' to a range 'r', then hiding
this
multicell range:

Sub Test3()
Dim c As Range
Dim r As Range
Dim t As Double
Dim i As Long
t = Timer
Application.ScreenUpdating = False
For i = 1 To 100
ActiveSheet.Range("A1:A40").EntireColumn.Hidden = False
Set r = Nothing
For Each c In ActiveSheet.Range("A1:A40").Cells
If Len(c.Value) = 0 Then
If r Is Nothing Then
Set r = c
Else
Set r = Union(r, c)
End If
End If
Next
r.EntireRow.Hidden = True
Next
Application.ScreenUpdating = True
Debug.Print Timer - t
End Sub

Five iterations averaged 1.6 seconds.

- Jon
-------
Jon Peltier, Microsoft Excel MVP
Peltier Technical Services, Inc.
http://PeltierTech.com/WordPress/
_______


"Monk" wrote in message
...
Hello

The macro below is taking an excessive amount of time to run. It takes
about
2 or 3 minutes to complete. Can someone please review the code and see
whether there is a way to speed it up?

Thanks

Monk


Application.ScreenUpdating = False
Columns("A:o").Select
Selection.EntireColumn.Hidden = False
Rows("10:10").Select
With Selection.Font
.Name = "Arial"
.Size = 10
.Strikethrough = False
.Superscript = False
.Subscript = False
.OutlineFont = False
.Shadow = False
.Underline = xlUnderlineStyleNone
End With
Selection.RowHeight = 30#
Rows("11:1251").Select
With Selection.Font
.Name = "Arial"
.Size = 10
.Strikethrough = False
.Superscript = False
.Subscript = False
.OutlineFont = False
.Shadow = False
.Underline = xlUnderlineStyleNone
End With
Selection.RowHeight = 12.75
Dim c As Range
For Each c In Range("A11:A1250")
If c.Value = "" Then
Rows(c.Row).Hidden = True
Else
Rows(c.Row).Hidden = False
End If
Next c
Range("I2").Select
Application.ScreenUpdating = True
End Sub







Dave Peterson

Macro Takes Long Time To Run
 
Jon had a typo in his code:
Set r = Union(r. c)
should have been:
Set r = Union(r, c)

But in your post, you used a comma. So I'm not sure if it's something else...

Monk wrote:

Thanks Jon. Sorry to be a pain but I am getting a "Compile Error: Argument
not optional" response on the Union (r,c) line

"Jon Peltier" wrote:

Monk -

I was showing a comparison between two techniques, a slow way and a fast
way. Each looped 100 times to give timer values that could be compared. You
took the slower code, and kept in the loop. So yeah, it's going to be slow.

What you need to do is replace this slow bit of your code:

Dim c As Range
For Each c In Range("A11:A1250")
If c.Value = "" Then
Rows(c.Row).Hidden = True
Else
Rows(c.Row).Hidden = False
End If
Next c

with this faster bit:

Dim c As Range
Dim r As Range
For Each c In Range("A11:A1250")
If c.Value = "" Then
If r is nothing then
Set r = c
else
Set r = Union(r. c)
endif
End If
Next c
r.EntireRow.Hidden = True

- Jon
-------
Jon Peltier, Microsoft Excel MVP
Peltier Technical Services, Inc.
http://PeltierTech.com/WordPress/
_______


"Monk" wrote in message
...
Hi Jon

I have used your code as shown below. I must have made an error somewhere
as
it still takes a couple of minutes to complete. Are you able to advise
where
my error is?

Monk


Application.ScreenUpdating = False
Application.Calculation = xlCalculationManual
Columns("A:o").EntireColumn.Hidden = False
With Rows(10).Font
.Name = "Arial"
.Size = 10
End With
Rows(10).RowHeight = 30
With Rows("11:1251").Font
.Name = "Arial"
.Size = 10
End With
Rows("11:1251").RowHeight = 12.75

Dim c As Range
Dim t As Double
Dim i As Long
t = Timer
For i = 1 To 100
ActiveSheet.Range("A11:A1250").EntireColumn.Hidden = False
For Each c In ActiveSheet.Range("A11:A1250").Cells
If c.Value = "" Then
c.EntireRow.Hidden = True
Else
c.EntireRow.Hidden = False
End If
Next
Next
Debug.Print Timer - t
Range("I2").Select
Application.Calculation = xlCalculationAutomatic
Application.ScreenUpdating = True
End Sub

"Jon Peltier" wrote:

I suspect the hiding of rows is your bottleneck, even though you use
Application.ScreenUpdating. Instead of hiding one row at a time, it is
faster to keep track of rows in a range, and hide the range all at once.

This is a shortened test version of your code:

Sub Test1()
Dim c As Range
Dim t As Double
Dim i As Long
t = Timer
Application.ScreenUpdating = False
For i = 1 To 100
ActiveSheet.Range("A1:A40").EntireColumn.Hidden = False
For Each c In ActiveSheet.Range("A1:A40").Cells
If c.Value = "" Then
c.EntireRow.Hidden = True
Else
c.EntireRow.Hidden = False
End If
Next
Next
Application.ScreenUpdating = True
Debug.Print Timer - t
End Sub

I ran this five times, with an average elapsed time of 5.8 seconds.

This does the same by adding each cell 'c' to a range 'r', then hiding
this
multicell range:

Sub Test3()
Dim c As Range
Dim r As Range
Dim t As Double
Dim i As Long
t = Timer
Application.ScreenUpdating = False
For i = 1 To 100
ActiveSheet.Range("A1:A40").EntireColumn.Hidden = False
Set r = Nothing
For Each c In ActiveSheet.Range("A1:A40").Cells
If Len(c.Value) = 0 Then
If r Is Nothing Then
Set r = c
Else
Set r = Union(r, c)
End If
End If
Next
r.EntireRow.Hidden = True
Next
Application.ScreenUpdating = True
Debug.Print Timer - t
End Sub

Five iterations averaged 1.6 seconds.

- Jon
-------
Jon Peltier, Microsoft Excel MVP
Peltier Technical Services, Inc.
http://PeltierTech.com/WordPress/
_______


"Monk" wrote in message
...
Hello

The macro below is taking an excessive amount of time to run. It takes
about
2 or 3 minutes to complete. Can someone please review the code and see
whether there is a way to speed it up?

Thanks

Monk


Application.ScreenUpdating = False
Columns("A:o").Select
Selection.EntireColumn.Hidden = False
Rows("10:10").Select
With Selection.Font
.Name = "Arial"
.Size = 10
.Strikethrough = False
.Superscript = False
.Subscript = False
.OutlineFont = False
.Shadow = False
.Underline = xlUnderlineStyleNone
End With
Selection.RowHeight = 30#
Rows("11:1251").Select
With Selection.Font
.Name = "Arial"
.Size = 10
.Strikethrough = False
.Superscript = False
.Subscript = False
.OutlineFont = False
.Shadow = False
.Underline = xlUnderlineStyleNone
End With
Selection.RowHeight = 12.75
Dim c As Range
For Each c In Range("A11:A1250")
If c.Value = "" Then
Rows(c.Row).Hidden = True
Else
Rows(c.Row).Hidden = False
End If
Next c
Range("I2").Select
Application.ScreenUpdating = True
End Sub







--

Dave Peterson

Jon Peltier

Macro Takes Long Time To Run
 
DOH!!

- Jon
-------
Jon Peltier, Microsoft Excel MVP
Peltier Technical Services, Inc.
http://PeltierTech.com/WordPress/
_______


"Dave Peterson" wrote in message
...
Jon had a typo in his code:
Set r = Union(r. c)
should have been:
Set r = Union(r, c)

But in your post, you used a comma. So I'm not sure if it's something
else...

Monk wrote:

Thanks Jon. Sorry to be a pain but I am getting a "Compile Error:
Argument
not optional" response on the Union (r,c) line

"Jon Peltier" wrote:

Monk -

I was showing a comparison between two techniques, a slow way and a
fast
way. Each looped 100 times to give timer values that could be compared.
You
took the slower code, and kept in the loop. So yeah, it's going to be
slow.

What you need to do is replace this slow bit of your code:

Dim c As Range
For Each c In Range("A11:A1250")
If c.Value = "" Then
Rows(c.Row).Hidden = True
Else
Rows(c.Row).Hidden = False
End If
Next c

with this faster bit:

Dim c As Range
Dim r As Range
For Each c In Range("A11:A1250")
If c.Value = "" Then
If r is nothing then
Set r = c
else
Set r = Union(r. c)
endif
End If
Next c
r.EntireRow.Hidden = True

- Jon
-------
Jon Peltier, Microsoft Excel MVP
Peltier Technical Services, Inc.
http://PeltierTech.com/WordPress/
_______


"Monk" wrote in message
...
Hi Jon

I have used your code as shown below. I must have made an error
somewhere
as
it still takes a couple of minutes to complete. Are you able to
advise
where
my error is?

Monk


Application.ScreenUpdating = False
Application.Calculation = xlCalculationManual
Columns("A:o").EntireColumn.Hidden = False
With Rows(10).Font
.Name = "Arial"
.Size = 10
End With
Rows(10).RowHeight = 30
With Rows("11:1251").Font
.Name = "Arial"
.Size = 10
End With
Rows("11:1251").RowHeight = 12.75

Dim c As Range
Dim t As Double
Dim i As Long
t = Timer
For i = 1 To 100
ActiveSheet.Range("A11:A1250").EntireColumn.Hidden = False
For Each c In ActiveSheet.Range("A11:A1250").Cells
If c.Value = "" Then
c.EntireRow.Hidden = True
Else
c.EntireRow.Hidden = False
End If
Next
Next
Debug.Print Timer - t
Range("I2").Select
Application.Calculation = xlCalculationAutomatic
Application.ScreenUpdating = True
End Sub

"Jon Peltier" wrote:

I suspect the hiding of rows is your bottleneck, even though you use
Application.ScreenUpdating. Instead of hiding one row at a time, it
is
faster to keep track of rows in a range, and hide the range all at
once.

This is a shortened test version of your code:

Sub Test1()
Dim c As Range
Dim t As Double
Dim i As Long
t = Timer
Application.ScreenUpdating = False
For i = 1 To 100
ActiveSheet.Range("A1:A40").EntireColumn.Hidden = False
For Each c In ActiveSheet.Range("A1:A40").Cells
If c.Value = "" Then
c.EntireRow.Hidden = True
Else
c.EntireRow.Hidden = False
End If
Next
Next
Application.ScreenUpdating = True
Debug.Print Timer - t
End Sub

I ran this five times, with an average elapsed time of 5.8 seconds.

This does the same by adding each cell 'c' to a range 'r', then
hiding
this
multicell range:

Sub Test3()
Dim c As Range
Dim r As Range
Dim t As Double
Dim i As Long
t = Timer
Application.ScreenUpdating = False
For i = 1 To 100
ActiveSheet.Range("A1:A40").EntireColumn.Hidden = False
Set r = Nothing
For Each c In ActiveSheet.Range("A1:A40").Cells
If Len(c.Value) = 0 Then
If r Is Nothing Then
Set r = c
Else
Set r = Union(r, c)
End If
End If
Next
r.EntireRow.Hidden = True
Next
Application.ScreenUpdating = True
Debug.Print Timer - t
End Sub

Five iterations averaged 1.6 seconds.

- Jon
-------
Jon Peltier, Microsoft Excel MVP
Peltier Technical Services, Inc.
http://PeltierTech.com/WordPress/
_______


"Monk" wrote in message
...
Hello

The macro below is taking an excessive amount of time to run. It
takes
about
2 or 3 minutes to complete. Can someone please review the code
and see
whether there is a way to speed it up?

Thanks

Monk


Application.ScreenUpdating = False
Columns("A:o").Select
Selection.EntireColumn.Hidden = False
Rows("10:10").Select
With Selection.Font
.Name = "Arial"
.Size = 10
.Strikethrough = False
.Superscript = False
.Subscript = False
.OutlineFont = False
.Shadow = False
.Underline = xlUnderlineStyleNone
End With
Selection.RowHeight = 30#
Rows("11:1251").Select
With Selection.Font
.Name = "Arial"
.Size = 10
.Strikethrough = False
.Superscript = False
.Subscript = False
.OutlineFont = False
.Shadow = False
.Underline = xlUnderlineStyleNone
End With
Selection.RowHeight = 12.75
Dim c As Range
For Each c In Range("A11:A1250")
If c.Value = "" Then
Rows(c.Row).Hidden = True
Else
Rows(c.Row).Hidden = False
End If
Next c
Range("I2").Select
Application.ScreenUpdating = True
End Sub







--

Dave Peterson





All times are GMT +1. The time now is 04:21 AM.

Powered by vBulletin® Copyright ©2000 - 2025, Jelsoft Enterprises Ltd.
ExcelBanter.com