0

the code below works 100%. It scans for a match in Column B and copies and renames a group of cells when a match is found. However the is a line For lRow = Sheets("HR-Calc").Cells(Cells.Rows.count, "b").End(xlUp).Row To 7 Step -1 Where the step -1 will scan row by row from the bottom of the sheet until a match is found. It would be much easier if the step was set to End.(xlUp) instead of -1. searching every row is overkill because of how the data is set up End.(xlUp) would massive cut down the run time. Is something like this possible?

Sub Fill_CB_Calc()

M_Start:

Application.ScreenUpdating = True

Sheets("summary").Activate
d_input = Application.InputBox("select first cell in data column", "Column Data Check", Default:="", Type:=8).Address(ReferenceStyle:=xlA1, RowAbsolute:=True, ColumnAbsolute:=False)

data_col = Left(d_input, InStr(2, d_input, "$") - 1)
data_row = Right(d_input, Len(d_input) - InStr(2, d_input, "$"))

Application.ScreenUpdating = False

Sheets("summary").Activate
Range(d_input).End(xlDown).Select

data_last = ActiveCell.Row

If IsEmpty(Range(data_col & data_row + 1)) = True Then
    data_last = data_row

Else
End If

    For j = data_row To data_last

CBtype = Sheets("summary").Range(data_col & j)

    Sheets("HR-Calc").Activate
    For lRow = Sheets("HR-Calc").Cells(Cells.Rows.count, "b").End(xlUp).Row To 7 Step -1

    If Sheets("HR-Calc").Cells(lRow, "b") = CBtype Then

            CBend = Sheets("HR-Calc").Range("C" & lRow).End(xlDown).Row + 1
            Sheets("HR-Calc").Rows(lRow & ":" & CBend).Copy

            CBstart = Sheets("HR-Calc").Range("c50000").End(xlUp).Row + 2

            ActiveWindow.ScrollRow = CBstart - 8

            Sheets("HR-Calc").Range("A" & CBstart).Insert Shift:=xlDown

            CBold = Right(Range("c" & CBstart), Len(Range("C" & CBstart)) - 2)

                box_name = Sheets("summary").Range(data_col & j).Offset(0, -10)

                CBnew = Right(box_name, Len(box_name) - 2) & "-"  ' <--this is custom and can be changed based on CB naming structure
                If CBnew = "" Or vbCancel Then
                End If
            CBend2 = Range("c50000").End(xlUp).Row - 2

            Range("C" & CBstart + 1 & ":" & "C" & CBend2).Select
                Selection.Replace What:=CBold & "-", Replacement:=CBnew, LookAt:=xlPart, _
                SearchOrder:=xlByRows, MatchCase:=False, SearchFormat:=False, _
                ReplaceFormat:=False

            Range("C" & CBstart).FormulaR1C1 = "CB" & Left(CBnew, Len(CBnew) - 1)

            GoTo M_Start2
    Else

    End If

Next lRow
M_Start2:
            Next j

YN_result = MsgBox("Fill info for another block/inverter?", vbYesNo + vbExclamation)
If YN_result = vbYes Then GoTo M_Start
If YN_result = vbNo Then GoTo jumpout

jumpout:
'    Sheets("summary").Range(d_input).Select
    Application.ScreenUpdating = True
End Sub
Community
  • 1
  • 1
Alberto Brown
  • 345
  • 1
  • 7
  • 24
  • 1
    Look at doing a `.find` from the bottom up. http://stackoverflow.com/questions/22464631/perform-a-find-within-vba-from-the-bottom-of-a-range-up – MatthewD Apr 13 '16 at 13:47
  • 1
    Also, using `.Select`/`.Activate` can slow down code. See [this thread](http://stackoverflow.com/questions/10714251/how-to-avoid-using-select-in-excel-vba-macros) on how to avoid using those. – BruceWayne Apr 13 '16 at 14:00
  • 1
    I'm having right trouble looking through your code - I copied and pasted it into my workbook and it asks to define these variables: `d_input, data_col, data_row, data_last, j, CBtype, lRow, cBend, CBstart, CBold, box_name, CBnew, CBend2, YN_result`. Good practice to add `Option Explicit` at the top of each module. You can get the value for `data_row` using `data_row = Range(d_input).Row`. When you use `End(xlDown)` are you looking for the last row or the first row containing no data? Should it always look at column B, or the column selected with `d_input`? As @MatthewD said - use `.Find`. – Darren Bartrup-Cook Apr 13 '16 at 14:06
  • @MatthewD if you could put that find suggestion as an answer I will be happy to accept it. It does what i need I just need to integrate it into my code. – Alberto Brown Apr 13 '16 at 17:30
  • 1
    @BruceWayne I tried to minimize my use of `select` and `activate`. That link is alos very helpful. When I have time I will go back thru my code and adjust accordingly. – Alberto Brown Apr 13 '16 at 17:32
  • @DarrenBartrup-Cook im looking from the bottom up, because of the loop that I'm running. find template ID, copy that template rename info in template, find next template repeat. as for the variables most of them are generated form user inputs or finding a row in the `HR-Calc` sheet – Alberto Brown Apr 13 '16 at 20:16

2 Answers2

1

I'm not sure if this will help but I've had a great performance increase with pulling the entire range you need to loop through into a variant array and then looping through the array. If I need to loop through large data sets, this method has worked out well.

Dim varArray as Variant
varArray = Range(....) 'set varArray to the range you're looping through
For y = 1 to uBound(varArray,1)  'loops through rows of the array
    'code for each row here
    'to loop through individual columns in that row, throw in another loop
    For x = 1 to uBound(varArray, 2) 'loop through columns of array
       'code here
    Next x
Next y

You can also define the column indexes prior to executing the loop. Then you only need to execute the you need to pull those directly in the loop.

'prior to executing the loop, define the column index of what you need to look at
Dim colRevenue as Integer
colRevenue = 5 'or a find function that searches for a header named "Revenue"

Dim varArray as Variant
    varArray = Range(....) 'set varArray to the range you're looping through
For y = 1 to uBound(varArray,1)  'loops through rows of the array
    tmpRevenue = CDbl(varArray(y, colRevenue))
Next y

Hope this helps.

David
  • 41
  • 5
  • 1
    This is a viable alternative but the loop should still step backwards; e.g. `For y = UBound(varArray,1) to LBound(varArray,1) Step -1` in this particular case. If the [Range.Find method](https://msdn.microsoft.com/en-us/library/office/ff839746.aspx) was isolated to a single column and **xlPrevious** was used, it will probably be as fast or slightly faster than the array loop. Either will be faster then looping through the worksheet's cells. –  Apr 14 '16 at 19:36
0

Look at doing a .find from the bottom up.

Perform a FIND, within vba, from the bottom of a range up

That will eliminate the need to do the for loop from the last row to the first occurrence of the value you want to locate.

Community
  • 1
  • 1
MatthewD
  • 6,719
  • 5
  • 22
  • 41