Page 1 of 2 12 LastLast
Results 1 to 15 of 19
  1. #1
    Star Lounger
    Join Date
    Jan 2001
    Location
    Iowa, USA
    Posts
    79
    Thanks
    0
    Thanked 0 Times in 0 Posts

    Speeding up this code (VBA/Excel/Win2K)

    I'm creating an on line order form that has over 400 lines of items that may be potentially ordered. To help the order receiver process the orders, I wrote some code that hides all rows that don't contain an odered item before sending the form. The code that does this is pretty simple but on my 500 MHz machine (a middle of the road sort of PC for most of the people who will be using the form) it takes about 2 minutes for the code to do its thing. Turning off screenupdating doesn't help...actually, it looks like the screen is still updating at each line hide.

    Here's the code I currently have...is there a better way of doing this?

    Thanks!
    Sue

    ScreenUpdating = False
    ' ----------------------------------------------------------------------
    ' Hide all rows with value of 0 in the K column
    ' ----------------------------------------------------------------------
    For rwIndex = 25 To 486
    ActiveSheet.Cells(rwIndex, 17).Select
    If ActiveCell.Value = "0" Then
    Selection.EntireRow.Hidden = True
    End If
    Next rwIndex

  2. #2
    Gold Lounger
    Join Date
    Feb 2001
    Location
    Dublin, Ireland, Republic of
    Posts
    2,697
    Thanks
    1
    Thanked 0 Times in 0 Posts

    Re: Speeding up this code (VBA/Excel/Win2K)

    The following should prove a bit faster :<pre>
    Dim oCell As Range
    For Each oCell In Range("K25:K486")
    If oCell.Value = 0 Then
    oCell.EntireRow.Hidden = True
    End If
    Next</pre>

    It would do no harm to maintain the screenupdating set to false

    Andrew C

  3. #3
    Star Lounger
    Join Date
    Jan 2001
    Location
    Iowa, USA
    Posts
    79
    Thanks
    0
    Thanked 0 Times in 0 Posts

    Re: Speeding up this code (VBA/Excel/Win2K)

    Andrew,

    This is very interesting...

    The code you suggest does run about 30 seconds faster (3/4 of the original time) but it doesn't run correctly. It doesn't hide a bunch of rows that should be hidden (and are appropriately hidden by the original code).

    In that respect, I can't see any reason why it should be different from the original code...

    Your code has the value without quotes and mine has it in quotes but if I change that in either code, no rows are hidden at all.

    In either case, I can't see that "screenupdating = false" has any effect. The display is still updated line by line as rows are hidden.

    Curiouser and curiouser....

    Sue

  4. #4
    Platinum Lounger
    Join Date
    Dec 2000
    Location
    Queanbeyan, New South Wales, Australia
    Posts
    3,730
    Thanks
    0
    Thanked 0 Times in 0 Posts

    Re: Speeding up this code (VBA/Excel/Win2K)

    Sue,

    I don't know whether this makes any difference to Andrew's code working or not- but your comment says hide based on the value in "K", but the the code says column 17. Does it make a difference to change Andrew's code from "K25:K486" to "Q25:Q486"?
    Subway Belconnen- home of the Signboard to make you smile. Get (almost) daily updates- follow SubwayBelconnen on Twitter.

  5. #5
    Gold Lounger
    Join Date
    Feb 2001
    Location
    Dublin, Ireland, Republic of
    Posts
    2,697
    Thanks
    1
    Thanked 0 Times in 0 Posts

    Re: Speeding up this code (VBA/Excel/Win2K)

    Sue,

    Curiouser and curiouser.... indeed.

    I think your code is operating on column Q (17), although you mentioned K. That might explain why different results are obtained from the 2 sets of code.

    I tried both sets of code on a Pentium I 200mhz, and with screen updating switched off, no mare than 1 or 2 seconds elapsed, depending on th enumber of rows to be hidden.

    It seems to me some other problem is happening in your case.

    With regard to screen updating ( it's effect should not be that great in this case) have you switched it off once before looping and then back on after the loop. Make sure the instruction is outside the loop. If you continue to have problems, please post the entire code, from Sub to End Sub.

    Andrew

  6. #6
    Plutonium Lounger
    Join Date
    Dec 2000
    Location
    Sacramento, California, USA
    Posts
    16,775
    Thanks
    0
    Thanked 1 Time in 1 Post

    Re: Speeding up this code (VBA/Excel/Win2K)

    Would it be feasible to filter the data to another sheet and only send the form with the filtered data? That way, you wouldn't need to hide anything.
    Charlotte

  7. #7
    Star Lounger
    Join Date
    Jan 2001
    Location
    Iowa, USA
    Posts
    79
    Thanks
    0
    Thanked 0 Times in 0 Posts

    Re: Speeding up this code (VBA/Excel/Win2K)

    Hmmm...there's a thought. I'll look into that.

    Thanks for the idea!
    Sue

  8. #8
    Star Lounger
    Join Date
    Jan 2001
    Location
    Iowa, USA
    Posts
    79
    Thanks
    0
    Thanked 0 Times in 0 Posts

    Re: Speeding up this code (VBA/Excel/Win2K)

    Ah!! That's it. Originally I used column K but switched to Q/17 and didn't edit the comment.

    Sue

  9. #9
    Star Lounger
    Join Date
    Jan 2001
    Location
    Iowa, USA
    Posts
    79
    Thanks
    0
    Thanked 0 Times in 0 Posts

    Re: Speeding up this code (VBA/Excel/Win2K)

    Andrew,

    You're right about the Q/K confusion. The comment is wrong...it should be Q.

    I don't know about the difference in time between your test and mine. My form was evaluating and hiding something over 400 rows. I basically left the whole form blank and let it hide all the rows in the 25 - 486
    range specified. I was hoping for a run time much more like yours...

    The screenupdating=false line is the first line after the Dim statements and it's never set to True so it's definitely not inside the loop.

    What else do you think might be causing the problem? Following is the entire code minus the usernames. I was timing only the first For...Next loop by putting a breakpoint on the For Each H... line.

    Thanks muchly!
    Sue

    ---------------------------------

    Sub SendFile()

    Dim rwIndex
    Dim BusCard As String
    Dim Imprint As String
    Dim HCells
    Dim PCells

    Imprint = ""


    ActiveSheet.Unprotect

    ScreenUpdating = False

    ' ----------------------------------------------------------------------
    ' Hide all rows with value of 0 in the Q column
    ' ----------------------------------------------------------------------
    For rwIndex = 25 To 486
    ActiveSheet.Cells(rwIndex, 17).Select
    If ActiveCell.Value = "0" Then
    Selection.EntireRow.Hidden = True
    End If
    Next rwIndex

    ' ----------------------------------------------------------------------
    ' If single-line imprints ordered, unhide the row below
    ' ----------------------------------------------------------------------
    HCells = Array(38, 41, 58, 139)
    PCells = Array(29, 49, 53, 61, 64, 116)

    For Each H In HCells
    ActiveSheet.Cells(H, 8).Select
    If ActiveCell.Value <> "0" Then
    ActiveSheet.Cells(H + 1, 8).Select
    Selection.EntireRow.Hidden = False
    End If
    Next H

    For Each P In PCells
    ActiveSheet.Cells(P, 16).Select
    If ActiveCell.Value <> "0" Then
    ActiveSheet.Cells(P + 1, 16).Select
    Selection.EntireRow.Hidden = False
    End If
    Next P


    ' ----------------------------------------------------------------------
    ' If any CommRes imprinted items ordered, set Imprint variable to True
    ' ----------------------------------------------------------------------
    For rwIndex = 69 To 74
    ActiveSheet.Cells(rwIndex, 17).Select
    If ActiveCell.Value <> "0" Then
    Imprint = "True"
    Exit For
    End If
    Next rwIndex

    ' ----------------------------------------------------------------------
    ' If CommRes imprint items ordered, unhide imprinted section w/ named
    ' range "Imprinted_Items"
    ' ----------------------------------------------------------------------
    If Imprint = "True" Then
    Application.Goto Reference:="Imprinted_Items"
    Selection.EntireRow.Hidden = False
    End If

    ' ----------------------------------------------------------------------
    ' send entire document to Country Store
    ' ----------------------------------------------------------------------
    ThisWorkbook.SendMail Recipients:="CS username goes here", _
    Subject:="Country Store Order from Web Site"

    ' ----------------------------------------------------------------------
    ' if order contains imprinted items, send them to
    ' CommRes contact
    ' ----------------------------------------------------------------------
    If Imprint = "True" Then
    Application.Goto Reference:="Imprinted_Items"
    Selection.EntireRow.Hidden = False
    Application.Goto Reference:="Non_Imprinted_Items"
    Selection.EntireRow.Hidden = True
    ThisWorkbook.SendMail Recipients:="CommRes contact goes here", _
    Subject:="Imprint Items Order from Web Site"
    End If

    ' ----------------------------------------------------------------------
    ' Clean up and don't save changes
    ' ----------------------------------------------------------------------
    ThisWorkbook.Close SaveChanges:=False

    End Sub

  10. #10
    Uranium Lounger
    Join Date
    Jan 2001
    Location
    South Carolina, USA
    Posts
    7,295
    Thanks
    0
    Thanked 0 Times in 0 Posts

    Re: Speeding up this code (VBA/Excel/Win2K)

    Try:

    <pre> Application.ScreenUpdating = False
    </pre>

    Without the "Application." you are just setting a variable named ScreenUpdating to False. I think that the following code will be about as fast as you can get:

    <pre>Dim oCell As Range
    Application.ScreenUpdating = False
    For Each oCell In Range("Q25:Q486")
    With oCell
    If .Value = 0 Then
    .EntireRow.Hidden = True
    End If
    End With
    Next
    Application.ScreenUpdating = True
    </pre>

    This ran in less that 1 second on my 400mz laptop hiding all of the rows from 25 to 486.
    Legare Coleman

  11. #11
    Star Lounger
    Join Date
    Jan 2001
    Location
    Iowa, USA
    Posts
    79
    Thanks
    0
    Thanked 0 Times in 0 Posts

    Re: Speeding up this code (VBA/Excel/Win2K)

    Legare,

    That "Application." on the screen updating did it. That change alone got the code runtime down to 5 seconds.

    I couldn't see any appreciable difference in speed otherwise between your code and mine but I implemented yours because I think it's more elegantly (if that's the right word...) written.

    Thanks for the help, everyone!!
    Sue

  12. #12
    Gold Lounger
    Join Date
    Feb 2001
    Location
    Dublin, Ireland, Republic of
    Posts
    2,697
    Thanks
    1
    Thanked 0 Times in 0 Posts

    Re: Speeding up this code (VBA/Excel/Win2K)

    Sue,

    The difference in speed would not be appreciable on 400 hundred rows or so, but if you test the For .. Next loop against the For Each construct using 1000s of rows the difference will be more noticeable.

    I overlooked your omission of Application. before screenupdating in your initial post. Sorry about that.

    Andrew

  13. #13
    Star Lounger
    Join Date
    Jan 2001
    Location
    Iowa, USA
    Posts
    79
    Thanks
    0
    Thanked 0 Times in 0 Posts

    Range vs Cells ???

    Andrew,

    Yep...I figured with a LOT more rows the speed difference would be more noticeable. I can't see this particular form growing to that many rows, though.

    One thing I noticed about the 2 versions of the code...

    When I use the For...Each construction Legare suggested, some rows are not correctly identified as having a non-0 value...whereas if I use my For...Next loop, they are. The ones that are not properly identified are rows where the P and Q cells of that row have been merged.

    Legare's code:
    ' For Each oCell In Range("Q25:Q486")
    ' With oCell
    ' If .Value = 0 Then
    ' .EntireRow.Hidden = True
    ' End If
    ' End With
    ' Next

    My code:
    For rwIndex = 25 To 482
    ActiveSheet.Cells(rwIndex, 17).Select
    If ActiveCell.Value = "0" Then
    Selection.EntireRow.Hidden = True
    End If
    Next rwIndex


    I suspect it's not a For...Each vs For...Next issue as much as it is a Range vs Cells issue.

    Does that seem reasonable? If so...is there a simple explanation for why the two are evaluated differently?

    Thanks!
    Sue

  14. #14
    Gold Lounger
    Join Date
    Feb 2001
    Location
    Dublin, Ireland, Republic of
    Posts
    2,697
    Thanks
    1
    Thanked 0 Times in 0 Posts

    Re: Range vs Cells ???

    Sue,

    What is the result if you use = 0 in both cases, as opposed to = "0" in your version. I would question the wisdom of evaluating a column of values, where in some cases the cell does not effectively exist, i.e. in merged cells.

    Try this<pre>
    Sub Test()
    Range("P1:Q1").Merge
    Range("Q1").Value = 100
    End Sub</pre>

    Andrew

  15. #15
    Star Lounger
    Join Date
    Jan 2001
    Location
    Iowa, USA
    Posts
    79
    Thanks
    0
    Thanked 0 Times in 0 Posts

    Re: Range vs Cells ???

    If I use "0" in the Range case, I get all the merged cells regardless of whether they contain a 0 or non-0 value. If I use 0, I get none of the merged cells, regardless of value. This is true whether I use Range("Q25:Q487") or Range(Cells(25, 17), Cells(487, 17)).

    If I use either "0" or 0 in the Cells case, I get all the merged cells that contain non-0 data and none of the ones that don't. Which is what I'm wanting...

    I didn't like having to evaluate merged cells, either, but I didn't have much choice (I don't think, anyway...) and the code worked the way I wanted it to. I wonder if something will "break" it somewhere down the line, though...

    I appreciate all the input...this has been a real learning experience for me!

    Sue

Page 1 of 2 12 LastLast

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •