How can I make this code faster? 2

[HorseGoose]

The worksheet has a grid containing raw materials, in order to format the worksheet correctly the first cell = "." if there is data in the row otherwise it is blank. The following code then goes through each row, if the row is blank it is hidden.

Other information, it is called from the activate sheet event and the screenupdating is set to false in there.

The code works fine, it is just glacially slow.


Sub bev_cogs_format()
' loops through the data lines which refer back the main beverage sheet
' if the line has nothing in it in terms of weight then it is hidden

' first unhide all the rows

Application.Calculation = xlCalculationManual
Application.EnableEvents = False

Range("C18:C44").Select
Selection.EntireRow.Hidden = False

' then loop through each row and see if there is any weight entered, if not then hide the row
Dim loops As Integer
For loops = 18 To 66
Select Case Range("C" & loops)
Case Is = "."
Range("C" & loops).Select
Selection.EntireRow.Hidden = True
Case Else
' do nothing - do not hide the line because there is a weight
End Select
Next loops

' adds username and date and time the cogs sheet was generetae
Range("N5") = Environ("Username")
Range("N6") = Now()

Application.Calculation = xlCalculationAutomatic
Application.EnableEvents = True

End Sub

 

[Rofeu]

some pointers:

where you have .Select and Selection, you can just knit them together, i.e. Range.Select Selection.DoSomething --> Range.DoSomething

first build a range that contains all the rows to be hidden, then hide. Take a look at Union to join up ranges.

Cheers,

Roel

 

[Chance1234]

Use a proper loop rather than a for next

Dim intStart as integer
Dim intCeiling as ineger 

intStart =18 
intCeiling = 88

Do until intStart =IntCeiling

'your code here 

intStart = intStart + 1

Loop

]

you also dont need teh row select in tehre either

Chance,

F, G + [sub]H[/sub]H

 

[HorseGoose]

Thanks for your inputs.

Makin the changes suggested it was the same speed. The issue seems to be the processing of the line of code which hides the row. Anyway, I speeded up by hiding everything then unhiding the rows with data, as there are less of them.

I tried for next, do while I also tried to replace select case with If Then but the speed was always the same.

No worries!

Thanks

 

[ClulessChris]

Don't know if it'll help but this may be worth a try:

    Application.ScreenUpdating = False
    Application.Calculation = xlCalculationManual
    Application.EnableEvents = False
    
    Range("C18:C44").EntireRow.Hidden = False

    Dim loops As Integer
    For loops = 18 To 66
        If Range("C" & loops) = "." Then
               Rows(loops & ":" & loops).EntireRow.Hidden = True
                
        End If
        
    Next loops
    
    Range("N5") = Environ("Username")
    Range("N6") = Now()
    
    Application.Calculation = xlCalculationAutomatic
    Application.EnableEvents = True
    Application.ScreenUpdating = True

Everybody is somebodys Nutter.

 

[Bong]

Instead of searching and hiding, why not just sort on the first column? Reverse order will put the blanks at the bottom.

_________________
Bob Rashkin

 

[SkipVought]

Why don't you just use an AutoFilter to hide rows????

Skip,
[sub]
When a group touring the Crest Toothpaste factory got caught in a large cooler, headlines read...
Tooth Company Freeze a Crowd! and
Many are Cold, but Few are Frozen![/sub]

 

[HorseGoose]

The output is to be printed and is automated within the application. It prints batch sheets for our factories to use. So the rows have to be hidden in order to get a good looking format. there is data underneath these rows which always needs to be present.

In addition the sheet can have many different formulas with different quantities of data in the lines.

Clueless, This is even slower as the If statment slows the code down even more.

Thanks to every one for commenting.

Lee.

 

[SkipVought]

...And AutoFilter???

FAST!!!!!!!!!!!!!!!!!

Skip,
[sub]
When a group touring the Crest Toothpaste factory got caught in a large cooler, headlines read...
Tooth Company Freeze a Crowd! and
Many are Cold, but Few are Frozen![/sub]

 

[Chance1234]

jsut a note from cluelesschris post

Application.Calculation = xlCalculationManual

are you using this line, because calculating formulas are the only thing that would be slowing down your process

Using a Do Loop is always faster then a for next,

Chance,

F, G + [sub]H[/sub]H

 

[StephenJR]

Listen to Skip!

 

[AnotherHiggins]

I'm just getting to the party, but when looking at the Original Post, my first thought was.... [!]Autofilter[/!]!!

Seriously.

There's no reason to use a loop for this. Loops just slow things down and should be avoided whenever a faster way is available.

Turn on your macro recorder - autofilter for Blanks in whatever column. Delete all visible rows.

Now that I think about it, if you are just deleting all rows where a single column in blank, you can do that with a single line of code and it might just be faster than even the autoFilter:

Columns("A:A").SpecialCells(xlCellTypeBlanks).EntireRow.Delete

(Change columns to whatever you want. )

[tt]_____
[blue]-John[/blue][/tt]
[tab][red]The plural of anecdote is not data[/red]

Help us help you. Please read FAQ 181-2886 before posting.

 

[firefytr]

Do/Loop vs For/Next won't give you a noticable difference

Do not Select

Turn off events*

AutoFilter is fast

Keeping Input data separate from Output data is always best

Do not loop where you do not have to, loops are killers



* You can use a starting call and ending call to a procedure...

sub ToggleEvents(blnState as boolean)
    With Application
        .Displayalerts = blnstate
        .enableevents = blnstate
        .screenupdating = blnstate
        If blnstate = true then
            .cutcopymode = not blnstate
            .statusbar = not blnstate
        end if
    end with
end sub

Call before/after routine, i.e. ...

Sub MyRoutine()
    Call ToggleEvents(False)
    'run your code here...
    Call ToggleEvents(True)
End sub

HTH

Regards,
Zack Barresse

Simplicity is the ultimate sophistication. What is a MS MVP? PODA
- Leonardo da Vinci

 

[xlbo]

Definitely go with an autofilter

Column C, criteria <> "."

Rgds, Geoff

We could learn a lot from crayons. Some are sharp, some are pretty and some are dull. Some have weird names and all are different colours but they all live in the same box.

Please read FAQ222-2244 before you ask a question

 

[firefytr]

Some time tests...

Option Explicit
Dim T As Double

Sub foo1()
    Application.Calculation = xlCalculationManual
    Application.EnableEvents = False
    Range("C18:C44").Select
    Selection.EntireRow.Hidden = False
    Dim loops As Integer
    For loops = 18 To 66
        Select Case Range("C" & loops)
        Case Is = "."
            Range("C" & loops).Select
            Selection.EntireRow.Hidden = True
        End Select
    Next loops
    Range("N5") = Environ("Username")
    Range("N6") = Now()
    Application.Calculation = xlCalculationAutomatic
    Application.EnableEvents = True
End Sub
Sub foo2()
    Dim arrCheck(), i As Long
    Call ToggleEvents(False)
    arrCheck = Range("C18:C66").Value
    For i = LBound(arrCheck) To UBound(arrCheck)
        If arrCheck(i, 1) = "." Then Rows(i + 18).Hidden = True
    Next i
    Call ToggleEvents(True)
End Sub
Sub foo3()
    Call ToggleEvents(False)
    With Range("C17:C66") 'assumes header row in row 17
        .AutoFilter field:=1, Criteria1:="."
        .SpecialCells(xlCellTypeVisible).EntireRow.Hidden = True
        .AutoFilter
    End With
    Call ToggleEvents(True)
End Sub

Sub TimeTestFooFoo1()
    T = Timer
    Call foo1
    Debug.Print "1:foo1:" & Timer - T
    Call foo1
    Debug.Print "2:foo1:" & Timer - T
    Call foo1
    Debug.Print "3:foo1:" & Timer - T
End Sub
Sub TimeTestFooFoo2()
    Call foo2
    Debug.Print "4:foo2:" & Timer - T
    Call foo2
    Debug.Print "5:foo2:" & Timer - T
    Call foo2
    Debug.Print "6:foo2:" & Timer - T
End Sub
Sub TimeTestFooFoo3()
    T = Timer
    Call foo3
    Debug.Print "7:foo3:" & Timer - T
    Call foo3
    Debug.Print "8:foo3:" & Timer - T
    Call foo3
    Debug.Print "9:foo3:" & Timer - T
End Sub

Sub TimeTestFooFooALL()
    Call TimeTestFooFoo1
    Call TimeTestFooFoo2
    Call TimeTestFooFoo3
End Sub

Sub ToggleEvents(blnState As Boolean)
    With Application
        .DisplayAlerts = blnState
        .EnableEvents = blnState
        .ScreenUpdating = blnState
        If blnState = True Then
            .CutCopyMode = Not blnState
            .StatusBar = Not blnState
        End If
    End With
End Sub

AutoFilter wins.

Results:

1:foo1:0.0625
2:foo1:0.140625
3:foo1:0.1875
4:foo2:1.65625
5:foo2:1.6875
6:foo2:1.703125
7:foo3:0.015625
8:foo3:0.046875
9:foo3:0.0625

Averages:

0.130208333

1.682291667

0.041666667

HTH

Regards,
Zack Barresse

Simplicity is the ultimate sophistication. What is a MS MVP? PODA
- Leonardo da Vinci

 

[firefytr]

Btw, that is ...

Original:
0.130208333

Array-based:
1.682291667

AutoFilter:
0.041666667

Regards,
Zack Barresse

Simplicity is the ultimate sophistication. What is a MS MVP? PODA
- Leonardo da Vinci

 

[AnotherHiggins]

Did you manage to speed things up, HorseGoose?

[tt]_____
[blue]-John[/blue][/tt]
[tab][red]The plural of anecdote is not data[/red]

Help us help you. Please read FAQ 181-2886 before posting.

 

[HorseGoose]

Hello,

This is the new code, it work like a rocket. Thanks for all your hellp, simplicity is indeed the best answer.

Sub bev_cogs_format()
    ' Formats the beverage COGS sheet for printing
    ' if the line has nothing in it in terms of weight then it is hidden
    
    Application.Calculation = xlCalculationManual
    Application.EnableEvents = False
    
    Range("C18:C65").AutoFilter ' ensure autofilter is off
    
    ' set autofilter to show rows with a in the selected column "."
    Range("C18:C65").AutoFilter field:=1, Criteria1:=".", VisibleDropDown:=False

    ' adds username and date and time the cogs sheet was generetae
    Range("N5") = Environ("Username")
    Range("N6") = Now()
    Range("H6").Select
    
    Application.Calculation = xlCalculationAutomatic
    Application.EnableEvents = True

End Sub

 

[firefytr]

Don't forget, if you want to turn AutoFilter off afterwards, just repeat the line of code without any particulars, i.e.

Range("C18:C65").AutoFilter

Glad it is working better for you.

Regards,
Zack Barresse

Simplicity is the ultimate sophistication. What is a MS MVP? PODA
- Leonardo da Vinci

 

[AnotherHiggins]

I had forgotten to give Zack a purple pointy thing - particularly for his post dated 16 Sep 07 14:44 with the procedure to toggle event states before / after code is run.

->

[tt]_____
[blue]-John[/blue][/tt]
[tab][red]The plural of anecdote is not data[/red]

Help us help you. Please read FAQ 181-2886 before posting.