VBA macro crashes when using variable Ranges instead of fixed ones

0

I've tried to include comments with the code to make it readable, especially as I'm sure there are better ways of doing this!

I have a table with purchases and sales for a specific product. E.g.

ID  PURCHASES PRICES SALES CALCULATED VALUE
1   X         X      X     X
1   X         X      X     X
1   X         X      X     X
2   X         X      X     X
2   X         X      X     X
2   X         X      X     X

I am trying to run a FIFO calculation whereby it calculates how many are sold per specific product. The calculation works fine overall, but I'm having difficulty making it run specifically for each product ID.

For each product I tried to select the calculated range based on finding where the product starts and the product ends as below, then select that range to do the calculation on.

However, when I use my startRow and endRow variables in a Range() function, I just get the application crashing.

When I use the numbers by hand, it works perfectly (although only for the product I've selected).

Do you have any advice as to what I am doing wrong with this? Also I'd be grateful for any tips how to improve my code!

Sub RowCount()
    Dim sell As Long
    Dim i As Integer
    Dim j As Integer
    Dim r As Integer
    Dim cnt As Long
    Dim sale As Long
    Dim startRow As Integer
    Dim endRow As Integer
    Dim cStage As Integer
    Dim pID As New Collection, ID
    Dim productIDs() As Variant
    Dim currProduct As Long
    Dim ar As Variant
    Dim Var As Variant

    'CLEAR PREVIOUS
    Range("G10:G65536").ClearContents

    'COLLECT ALL PRODUCTS
    productIDs() = Range("B10", Range("B65536").End(xlUp)) 'IDs

    On Error Resume Next
    For Each ID In productIDs
        pID.Add ID, ID
    Next

    'CALCULATE SALES FOR EACH PRODUCT
    For currProduct = 1 To pID.Count
        '
        ' FIND START AND END ROW FOR currProduct
        '
        cStage = 0 'searching for the first row
        'calculate start and end row numbers for product
        For r = 1 To Rows.Count 'for each row
            If pID(currProduct) = Range("B" & r) And cStage = 0 Then 'found first of current product ID
                startRow = r 'first row is current row
                cStage = 1 ' moving to searching for the end row
            ElseIf pID(x) <> Range("B" & r) And cStage = 1 Then 'found the first row and now passed the final row
                cStage = 2 'search no more
                endRow = r - 1 'final product was previous row
            End If
        Next r

        'Working (for product number 2)
        ar = Range("C14:C19") 'Purchases
        Var = Range("D14:D19") 'Prices

        'Not Working
        'ar = Range("C" & startRow, Range("C" & endRow).End(xlUp)) 'Purchases
        'Var = Range("D" & startRow, Range("D" & endRow).End(xlUp)) 'Prices

        '
        ' PERFORM CALCULATIONS ON PRODUCT SALES/EACH ROW
        ' WORKS WHEN currProduct's RANGE IS CORRECT
        '
        For i = 10 To Range("A" & Rows.Count).End(xlUp).Row
            If pID(currProduct) = Range("B" & i) Then
                sell = Range("E" & i)
                sale = 0
                j = 1
                Do While sell > 0 And pID(currProduct) = Range("B" & i)
                    cnt = ar(j, 1)
                    ar(j, 1) = IIf(ar(j, 1) > sell, ar(j, 1) - sell, 0) 'iif
                    sell = sell - (cnt - ar(j, 1))
                    sale = sale + (cnt - ar(j, 1)) * Var(j, 1)
                    j = j + 1
                Loop
                Range("G1000").End(xlUp)(2) = sale 'output the sales
            End If
        Next i 'next sale
    Next currProduct 'next product
End Sub

Lawrence

Posted 2015-09-02T09:41:34.363

Reputation: 11

Idea: Try to reduce the range and check if it still crashes. At Range("G10:G65536").ClearContents and productIDs() = Range("B10", Range("B65536").End(xlUp)) – Divin3 – 2015-09-02T09:58:10.700

anyway, You should consider implementing the UsedRange.Columns instead. Learn more at: http://www.thespreadsheetguru.com/blog/2014/7/7/5-different-ways-to-find-the-last-row-or-last-column-using-vba

– Divin3 – 2015-09-02T10:29:01.210

An On Error Resume Next without an On Error goto 0 is not good practice. Add it after the command which can produce errors. Also: Without a working Example sheet filled with data, I'm not able to reproduce anything and we can't help you further – nixda – 2015-09-02T19:16:09.203

Answers

0

You could also create Ranges using the Cells method of the worksheet:

Set ar = sheet.Range(sheet.Cells(startrow, 3), sheet.Cells(endrow, 3))

This syntax gives the Range function a start point and an end point, each of which is a single cell in the worksheet. The column is not specified as a letter here but as the respective index, hence the 3. I would recommend to explicitly specify the worksheet on which you want to create, or work with, the desired range. So prefer

Set ar = sheet.Range(...)

over

Set ar = Range(...)

To me, the latter reads like Excel is creating the range out of thin air. If you give VBA just the Range keyword, it probably relates to the active worksheet but there is no way of knowing whether this is reliable and will keep working like that in all versions to come. If you want to target the active worksheet with your ranges, use this:

Set ar = ActiveSheet.Range(...)

Hope it helps ;o)

JoeMuc2008

Posted 2015-09-02T09:41:34.363

Reputation: 69

0

Ranges must be assigned to variables using the Set keyword.

Set ar = Range("C" & startrow, Range("C" & endrow).End(xlUp))

Kyle

Posted 2015-09-02T09:41:34.363

Reputation: 2 286