0

I've developed a macro to cleanup sets of copy/pasted data that is just a series of rows into an Excel with multiple columns and headers. To help clean the data I've added three FOR LOOP processes to:

  1. Delete the rows that were numbered bullets
  2. Deleted data that was extraneous (miles & minActive)
  3. Manually cut/paste the other dataset into their respective columns (steps & stepavg)

It seems to work well however I'd like to optimize the process. While running the macro, the issue is that I used "UsedRange" to figure out how many rows are present for FOR LOOP 1 (approx 800 rows). During that loops, many rows are deleted so it may filter from 800 to 350. Then when FOR LOOP 2 is performed, it seems like UsedRange is still reference 800 rows so the loop just continues, which filters from 350 to 65. Finally with FOR LOOP 3, it crunches and completes all 65 successfully and I can tell that its done. BUT, it'll keep going to row 800!

Any suggestions to "clear" or "reset" the UsedRange so this process is faster? Other than this issue, my macro works great.

'Cleaning the Data
    Dim i As Long
    Dim j As Long
    Dim k As Long
    Dim maxRow As Long

    maxRow = ActiveSheet.UsedRange.Rows.Count

    'Removes all those single number rows
    For i = 2 To maxRow Step 3
        Rows(i).Select
        Selection.Delete Shift:=xlLeft
    Next i
    Range("A1").Select

    'Removes all those miles and min active data
    Dim maxRow2 As Long
    maxRow2 = ActiveSheet.UsedRange.Rows.Count
    For j = 5 To maxRow2 Step 3
        Range(Rows(j), Rows(j + 5)).Select
        Selection.Delete Shift:=x1Up
    Next j
    Range("A1").Select

    'Cut/paste the Steps and StepsAvg data
    Dim maxRow3 As Long
    maxRow3 = ActiveSheet.UsedRange.Rows.Count
    For k = 3 To maxRow Step 1
        Cells(k, 1).Select
        Selection.Cut
        Cells(k - 1, 2).Select
        ActiveSheet.Paste
        Cells(k + 1, 1).Select
        Selection.Cut
        Cells(k - 1, 3).Select
        ActiveSheet.Paste
        Range(Rows(k), Rows(k + 1)).Select
        Selection.Delete Shift:=x1Up
    Next k
YowE3K
  • 23,852
  • 7
  • 26
  • 40
joshjayse
  • 77
  • 2
  • 2
  • 16
  • To *make it faster*, the first thing to do is [avoid using select](https://stackoverflow.com/questions/10714251/how-to-avoid-using-select-in-excel-vba-macros) – A.S.H Jun 02 '17 at 22:49
  • Thanks, I have begun to start improving the code. Do you have a suggestion to replace the "ActiveSheet.Paste"? WAS: "Range("A1:C1").Select... Selection.Cut... Range("B1").Select... ActiveSheet.Paste" TRIED: "Range("A1:C1").Cut... Range("B1").ActiveSheet.Paste" but did not compile Any suggestions? – joshjayse Jun 02 '17 at 23:00
  • Do you want just the values, or to paste the format too? – BruceWayne Jun 02 '17 at 23:50
  • For starters, .UsedRange is unreliable and sometimes unpredictable. Don't you have one column that will always have values down to the bottom of the UsedRange? Don't you have something like a header row that will not have any blanks to the end of the UsedRange? –  Jun 03 '17 at 00:27
  • In the code that follows *'Removes all those single number rows* I'm unclear on whether you know that you are deleting one of the rows you are using to count with or not. Typically you would work from the bottom up when deleting rows so that you do not step over a row that was substituted after a row was deleted. –  Jun 03 '17 at 00:37
  • ´maxrow=cells(rows.count,"a").end(xlup).rows´. Also when deleting rows, one should go from max to 1, because if you delete row4, then row5 becomes 4, but your next for loop is 5. – Patrick Lepelletier Jun 03 '17 at 00:47
  • @BruceWayne just the values. @ jeeped yes I do have a header row. I'm not 100% sure what UsedRange really does, but I added it from a similar question and worked as needed. Also for deleting single number rows, that is referring to the numbered bullet data (i.e. 1), data1, data2, data3, 2), data4, data5, etc. Seems to work as desired. Would bottom-up optimize the process? @ patrick, yes but that is the intention. I will try that maxrow formula. – joshjayse Jun 09 '17 at 18:33

1 Answers1

1

For a start here are a few pointers:

1) Don't use a variable unless you are going to use it more than once. maxRow, maxRow2 and maxRow3 are pointless. Each new variable increases the amount of memory (RAM) your application is using. At some point the VBA 'garbage collector' will come along and slow your code even more. Using numerous variables is bad practice for this reason, but reducing the number of these unnecessary variables also improves readability and reduces clutter.


2) When defining a loop variable there is no need to define 3 separate variables for each loop. Use the same one throughout. The only reason to use more than 1 loop variable is if you are using nested loops. E.G.

for i = 1 to 10
    for j = 2 to 5
        for k = 3 to 7
            debug.print i & "," & j & "," & k
        next k
    next j
next i

3) You don't need to select a cell to manipulate it. The following statements are equivalent:

cells(1,1).select
selection.value = "hello"

v.s.

cells(1,1).value = "hello"

Although in this case the speed is negligible, while looping over a region, setting the value directly is much faster than selecting the cell and then setting the value.


4) To transfer a value from one cell to another you do not need to use Cut and Paste:

cells(1,1).cut
cells(2,1).select
activesheet.paste

is (almost) the same as

cells(2,1).value = cells(1,1).value  'doesn't copy formatting or formula! I assume this isn't required.

5) Shift isn't required when deleting whole rows of data (unless you want to shift the data down). So rows(i).delete Shift:=x1Up can be simplified to rows(i).delete


Throwing all that together you get this, which in my opinion is a lot more readable and is also a lot faster:

Dim i As Long

'Removes all those single number rows
For i = 2 To ActiveSheet.UsedRange.Rows.Count Step 3
    Rows(i).Delete
Next i

'Removes all those miles and min active data
For i = 5 To ActiveSheet.UsedRange.Rows.Count Step 3
    Range(Rows(i), Rows(i + 5)).Delete
Next i

'Cut/paste the Steps and StepsAvg data
For i = 3 To ActiveSheet.UsedRange.Rows.Count Step 1
    Cells(i - 1, 2).Value = Cells(i, 1).Value
    Cells(i - 1, 3).Value = Cells(i + 1, 1).Value
    Range(Rows(i), Rows(i + 1)).Delete
Next i

There is still a lot that can be done to improve the speed. For example Application.ScreenUpdating=false and Application.EnableEvents=False. More complex efficiencies include deleting rows in bulk instead of row by row. E.G.

'Removes all those single number rows
Dim rng as range: set rng = Rows(2)
For i = 5 To ActiveSheet.UsedRange.Rows.Count Step 3
    set rng = Application.union(rng,Rows(i))
Next i
rng.Delete

Although the code looks more complex it is also faster because you are doing more rows in bulk. Another alternative would be to use array instead of ranges which would likely be the fastest method, but would be vastly more complex.

dim myArray as variant: myArray = Activesheet.UsedRange.Value
'do stuff with array
ActiveSheet.Clear
ActiveSheet.range(cells(1,1),cells(ubound(myArray,1),ubound(myArray,2)).value = myArray

But I digress the former is likely fast enough for you as is.

Sancarn
  • 2,575
  • 20
  • 45
  • You kind sir, are a GENIUS! Implementing these efforts greatly reduced processing time. I have yet to try the complex efficiencies but this has already worked wonders. – joshjayse Jun 09 '17 at 21:23
  • The complex efficiencies aren't totally necessary. I did test your original code and it did take a looooong time. I felt your pain... Glad I could help! – Sancarn Jun 09 '17 at 21:40