 |
 | Home | Contact Us | FAQ | Search & Site Map | Link to Us |
 | Sign In | Join | Other 45 Sites in Network |
| HomeDiscussionsAccessExcelInfoPathOutlookPowerPointPublisherWordDirectoryUser Groups Related Topics Outlook ExpressInternet ExplorerWindowsMS Server ProductsMore Topics ... |
MS Office Forum / Excel / Programming / March 2008Code runs to slow
We have a spreadsheet into which we dump a large amount of data (averaging around 55,000+ rows). We then use formulas to detect error in the data so that we can go back into the original program and correct those errors. What the code below does (my thanks to Rick Rothstein, MVP -VB) is hides all rows which do not have errors leaving only the rows with errors (saves one from having to scroll through more than 55,000 rows in search of errors). My problem is that it takes to long to hide the rows (in excess of 5 minutes). Does anyone have any thoughts on how to speed up the process? Sub HideRowIfZeroInG() ' ' Application.ScreenUpdating = False Dim R As Range Dim LastRow As Long With Worksheets("Negative Miles and Missing Perf") LastRow = .Cells(Rows.Count, "J").End(xlUp).Row If LastRow > 65536 Then LastRow = 65536 For Each R In .Range("J3:J" & CStr(LastRow)) If R.Value = 0 And R.Value <> "" Then R.EntireRow.Hidden = True Next End With Application.ScreenUpdating = True End Sub Have you thought about just using Data|Filter|Autofilter the range (column J) and showing all the rows that don't have the cell in column J equal to 0? > We have a spreadsheet into which we dump a large amount of data (averaging > around 55,000+ rows). We then use formulas to detect error in the data so [quoted text clipped - 24 lines] > > End Sub  SignatureDave Peterson Thank you sir. Having never used Filters before, I spent some time in the help files learning how to use Filters, and I have to say it is much fast. > Have you thought about just using Data|Filter|Autofilter the range (column > J) [quoted text clipped - 32 lines] >> >> End Sub I know you have a different solution now, but your comment about the speed of the code I posted for you earlier got me to thinking. Probably the speed problem is due to the continual hiding of the rows one-by-one. I'm thinking the code below should be more efficient. If you have the chance, I would be interested in how fast the code below is compared to the code I gave you earlier. Sub HideRowIfZeroInJ() Dim R As Range Dim RowsToHide As Range Dim LastRow As Long Application.ScreenUpdating = False With Worksheets("Sheet3") LastRow = .Cells(Rows.Count, "J").End(xlUp).Row For Each R In .Range("J3:J" & CStr(LastRow)) If R.Value = 0 And R.Value <> "" Then If RowsToHide Is Nothing Then Set RowsToHide = R Else Set RowsToHide = Union(R, RowsToHide) End If End If Next If Not RowsToHide Is Nothing Then RowsToHide.EntireRow.Hidden = True End With Application.ScreenUpdating = True End Sub Rick > Thank you sir. > [quoted text clipped - 38 lines] >>> >>> End Sub It went from 8 minutes 42 seconds to only 4 seconds. I would say that is a very dramatic improvement (with 65536 rows involved). >I know you have a different solution now, but your comment about the speed >of the code I posted for you earlier got me to thinking. Probably the speed [quoted text clipped - 69 lines] >>>> >>>> End Sub Yeah, I figured it might come out just a tad faster.<g> Thanks for carrying out the experiment for me. Rick > It went from 8 minutes 42 seconds to only 4 seconds. I would say that > is a very dramatic improvement (with 65536 rows involved). [quoted text clipped - 73 lines] >>>>> >>>>> End Sub Your code works so well I am thinking for reverting back to it, since it allows me to run a routine to unhide rows (which works equally fast) my problem is this code does not work when there are formulas in row J. The formulas in the cells return either a "" value or 1 of 3 text values (GPS Error, Missing Perform Time, Negative Miles). Is there any way to make it work with the formulas in column J? > Yeah, I figured it might come out just a tad faster.<g> Thanks for > carrying out the experiment for me. [quoted text clipped - 78 lines] >>>>>> >>>>>> End Sub If the code you have now is working faster than mine, then keep it. You should be able to unhide all the hidden rows with this single line... Worksheets("Sheet1").Range("A:A").EntireRow.Hidden = False After you do that, the last row or contiguous rows of previously hidden rows will remain selected. You can either clear the selection like this... Application.Selection.Clear but that will leave the active cell at this "last row" which could be way down on your worksheet. So, alternately, you can simply move the active cell to a cell higher up in the worksheet; something like this... Worksheets("Sheet1").Cells(1, 1).Select I'm not sure if any of the above would be aided by turning off ScreenUpdating beforehand and back on afterwards or not. Rick > Your code works so well I am thinking for reverting back to it, since it > allows me to run a routine to unhide rows (which works equally fast) my [quoted text clipped - 86 lines] >>>>>>> >>>>>>> End Sub Actually, Peter T shows a much better method in the demo code (about my Union method) that he posted elsewhere in this thread... simply execute this single line of code to unhide every hidden cell (and leave the active cell exactly where it currently is and with no selected ranges to deal with)... Rows.Hidden = False Rick > If the code you have now is working faster than mine, then keep it. You > should be able to unhide all the hidden rows with this single line... [quoted text clipped - 110 lines] >>>>>>>> >>>>>>>> End Sub One thing to watch out for with Union is that it becomes exponentially slower if/as the number of discontiguous areas in the unioned range increase. Typically it's faster to process batches of 50-400 areas at a time. The exact qty of areas depends on what the process is but a good rule of thumb is limit to about 80-100. The following should demonstrate the difference, trying to union 20k areas is not viable. Sub UnionDemo() Dim bFastTest As Boolean Dim bGotRng As Boolean Dim i As Long Dim arr() As Long Dim rHide As Range Rows.Hidden = False Columns(1).Clear Set rng = Range("A1:A40000") ReDim arr(1 To rng.Rows.Count, 1 To 1) For i = 1 To UBound(arr) Step 2 arr(i, 1) = 1 Next rng.Value = arr bFastTest = True ' bFastTest = False ' reduce rng size above to say 4k or less for testing t = Timer ' sensitive timer not required ! If bFastTest Then For Each cell In rng If cell.Value = 0 Then If bGotRng Then Set rHide = Union(rHide, cell) If rHide.Areas.Count > 100 Then rHide.EntireRow.Hidden = True Set rHide = Nothing bGotRng = False End If Else Set rHide = cell bGotRng = True End If End If Next If Not rHide Is Nothing Then ' or If bGotRng ... rHide.EntireRow.Hidden = True End If Else ' do ctrl-break if it's taking to long Set rHide = Nothing For Each cell In rng If cell.Value = 0 Then If rHide Is Nothing Then Set rHide = cell Else Set rHide = Union(rHide, cell) End If End If Next If Not rHide Is Nothing Then rHide.EntireRow.Hidden = True End If End If Debug.Print Timer - t End Sub Regards, Peter T > I know you have a different solution now, but your comment about the speed > of the code I posted for you earlier got me to thinking. Probably the speed [quoted text clipped - 25 lines] > > Rick <snip> I got a difference of 1.18359375 seconds when I ran your code; but I have a pretty fast computer here, so that time differential may not be representative. In any case, point taken... limit the unionizing to groups of 100 or so... even doing that should still produce an enormous improvement over doing it in the straight forward, one-at-a-time iteration approach. Thanks for pointing that out. Rick > One thing to watch out for with Union is that it becomes exponentially > slower if/as the number of discontiguous areas in the unioned range [quoted text clipped - 112 lines] >> > <snip> Ignore what I said about my timing... I misunderstood your posting and thought your code measured the time-difference in a single run... I didn't realize it was a two-step process requiring the bFastTest be flipped manually for the two cases... the time I reported was the time it took to run the "chunk" method. I had to break into the "non-chunk" method as it was taking too long. Again, thanks for pointing this problem out. By the way, for those out there following this thread, the reason all this is important is the Union technique has application to deleting rows of data (a relatively common question in these groups) and the apparently recommended method is to iterate the data rows, from the last data row to the first, deleting rows as you go... this Union technique (still requiring the backward iterations if using Peter T's "chunk" method) promises to be an enormously quicker approach, so it is important to fine-tune it as much as is possible. Rick >I got a difference of 1.18359375 seconds when I ran your code; but I have a >pretty fast computer here, so that time differential may not be [quoted text clipped - 122 lines] >>> >> <snip> "Rick Rothstein (MVP - VB)" wrote in message > Ignore what I said about my timing... I misunderstood your posting and > thought your code measured the time-difference in a single run... I didn't > realize it was a two-step process requiring the bFastTest be flipped > manually for the two cases... the time I reported was the time it took to > run the "chunk" method. I had to break into the "non-chunk" method as it was > taking too long. Yes I can see now it was not clear how to test. Although normally in such a test routine it would compare first one way then the other, but in this case for most systems it would not be possible to do the second test which would involve making a union of 20k areas. Hence the comment in the routine to reduce the range before testing the 'big' union method. Regards, Peter T PS more posts crossing - in future I will wait at least 5 minutes before responding <g> Was that making a union of 2k areas (loop of 4000 cells) or 20k areas (40k cells). If the if the latter I can only assume your machine runs on a combination of nitrous-oxide and steroids. For my curiosity how long for you to run this - Sub SteroidTest() Dim i As Long Dim t As Single Dim rng As Range t = Timer For i = 1 To 40000 Step 2 If rng Is Nothing Then Set rng = Cells(i, 1) Else Set rng = Union(rng, Cells(i, 1)) End If Next Debug.Print Timer - t ' ? Debug.Print rng.Areas.Count ' 20,000 End Sub Regards, Peter T > I got a difference of 1.18359375 seconds when I ran your code; but I have a > pretty fast computer here, so that time differential may not be [quoted text clipped - 121 lines] > >> > > <snip> My turn to say ignore that - our posts crossed. Regards, Peter T "Peter T" <peter_t@discussions> wrote in message > Was that making a union of 2k areas (loop of 4000 cells) or 20k areas (40k > cells). If the if the latter I can only assume your machine runs on a > combination of nitrous-oxide and steroids. For my curiosity how long for you > to run this - <snip> I see you haven't read my follow up post to the one you just responded to yet... Rick > Was that making a union of 2k areas (loop of 4000 cells) or 20k areas (40k > cells). If the if the latter I can only assume your machine runs on a [quoted text clipped - 158 lines] >> >> >> > <snip> |
Reply to this Message |
 Sign In
 Join
 My Latest Posts My Monitored Threads My Blog My Photo Gallery My Profile My Homepage Start New Thread Enable EMail Alerts Rate this Thread
|
©2008 Advenet LLC Privacy Policy - Terms of Use
This website includes both content owned or controlled by Advenet as well as content owned or controlled by third parties.