1

I am trying to do a simple loop, where I have declared some variables as array entries. I originally used them as variables to be overwritten, but changed it when I read that these variables will not automatically overwrite each time through the loop.

My problem is that this loop terminates after the first iteration (with no error). I can't seem to figure out why...

The code is essentially to find cons_sum(i,2) for each i, or row in the Pre-Summary sheet and sum some data in another sheet BOPE, then insert that sum into Pre-Summary.

This is my first post and I am teaching myself vba so please excuse any code fails.

This is my code:

Option Explicit

Sub Create_GAR080()
    Consmonth = Sheets("GAR080").Range("B2").Value

    Sheets("Pre-Summary").Select

    LastRow_summary = Cells(Rows.Count, "A").End(xlUp).Row
    LastRow = 156
    LastCol = 16

    Dim cons_sum() As Variant
    ReDim cons_sum(LastRow_summary, 4)

    For i = 1 To LastRow_summary Step 1

        cons_sum(i, 1) = Cells(i, 2).Value & "" 'pulls participant
        cons_sum(i, 2) = cons_sum(i, 1) & Cells(i, 1) ' participant and gas gate concatenated

        If cons_sum(i, 1) = "BOPE" Then
             Sheets(cons_sum(i, 1)).Select

             cons_sum(i, 3) = WorksheetFunction.Match(cons_sum(i, 2), Sheets(cons_sum(i, 1)).Range("A:A")) ' find participant gas gate combo

             cons_sum(i, 4) = Application.Sum(Sheets(cons_sum(i, 1)).Range(Cells(cons_sum(i, 3), 5), Cells(cons_sum(i, 3), 16)))

             If cons_sum(i, 4) > 0 Then
                Sheets("Pre-Summary").Cells(i, 4).Value = cons_sum(i, 4)
             End If
        End If
    Next i

    On Error Resume Next
End Sub
Siddharth Rout
  • 147,039
  • 17
  • 206
  • 250
  • 1
    Do a `debug.print` on `Lastrow_summay` before the loop. I suspect you are looping over the number of rows between row A and the top row.. that is, looping over the first row. – Bingo Feb 18 '13 at 04:33
  • Hi Bingo,LastRow_Summary comes out as 1848, which is what I expected.. – user2081943 Feb 18 '13 at 04:48
  • 3
    One straight problem that I am seeing is the use of `Select` When you say `Sheets(cons_sum(i, 1)).Select`, you change the sheet because of which your `cons_sum(i, 1) = Cells(i, 2).Value` will refer to the wrong sheet. I would recommend having a look at this link. You should really avoid the use of `.Select` http://stackoverflow.com/questions/10714251/excel-macro-avoiding-using-select Also pleassssssssssssssssssssse avoid using `On Error Resume Next` for unnecessary reasons :) – Siddharth Rout Feb 18 '13 at 07:04

2 Answers2

1

As Siddharth points out, your changing of sheets will result in cons_sum(i, 1) = "BOPE"to always be negative. Therefore, while the loop will run 1,848 times, it will not change anything.

In addition, a few more remarks:

  • You are using a 1848x4 array to do multiple operations in each line - but only stored the value of each operation and not use the array afterwards. Thus, you do not need 1848x4, but only 1x4, as you can reuse the variables
  • Instead of using an array, it is much better to use speaking variables. This will make your code much easier to understand
  • You have a lot of assumptions about the workbook structure hidden in the VBA formulas/statements, e.g. the number of rows, the name "BOPE", etc.. It is better to store them in constants in the beginning of the macro - or even better to store them somewhere on a settings sheets and refer to them with named ranges
  • You most likely forgot FALSE (or 0) as the third parameter for your match function. Therefore the function might return the wrong values if the column is not sorted
  • Instead of Range(Cells(x1,y1),Cells(x2,y2)) you can use Range.Resize and Range.Offset (and combinations thereof). This makes the code much easier to read!
  • do not use On Error Resume Next unless you know exactly what error you are happy to neglect! Even if you use it, use another On Error statement right after the operation that is allowed to produce an error.

Taking this into account, I reworked your code to the following:

Sub Create_GAR080_reworked()
    Const cStrTerm As String = "BOPE"

    Dim wsData As Worksheet
    Dim lngRowCount As Long, i As Long
    Dim strParticipantGasID As String
    Dim lngParticipantGasCombo As Long
    Dim dblSum As Double

    Set wsData = Sheets(cStrTerm)

    With Sheets("Pre-Summary")
        lngRowCount = .Cells(Rows.Count, 1).End(xlUp).Row
        For i = 1 To lngRowCount

            If .Cells(i, 2) = cStrTerm Then
                strParticipantGasID = cStrTerm & .Cells(i, 1)   ' participant and gas gate concatenated

                lngParticipantGasCombo = WorksheetFunction.Match( _
                    strParticipantGasID, wsData.Range("A:A"), 0)  ' find participant gas gate combo

                dblSum = Application.Sum( _
                    wsData.Range("E1:P1").Offset(lngParticipantGasCombo - 1))

                If dblSum > 0 Then
                   .Cells(i, 4).Value = dblSum
                End If
            End If
        Next i
    End With
End Sub

As I do not have your worksheet, I couldn't debug it. Also, not sure if I hit the right names as I have no idea what each variable is really refer to. But it should give you a start.

Peter Albert
  • 16,917
  • 5
  • 64
  • 88
0

If the first itiration the code Sheets(cons_sum(i, 1)).Select is hit, you'll never get back to the Pre-Summary sheet.

Try:

Option Explicit

Sub Create_GAR080()
    Consmonth = Sheets("GAR080").Range("B2").Value

    Sheets("Pre-Summary").Select

    LastRow_summary = Cells(Rows.Count, "A").End(xlUp).Row
    LastRow = 156
    LastCol = 16

    Dim cons_sum() As Variant
    ReDim cons_sum(LastRow_summary, 4)

    For i = 1 To LastRow_summary Step 1
        Sheets("Pre-Summary").Select

        cons_sum(i, 1) = Cells(i, 2).Value & "" 'pulls participant
        cons_sum(i, 2) = cons_sum(i, 1) & Cells(i, 1) ' participant and gas gate concatenated

        If cons_sum(i, 1) = "BOPE" Then
             Sheets(cons_sum(i, 1)).Select

             cons_sum(i, 3) = WorksheetFunction.Match(cons_sum(i, 2), Sheets(cons_sum(i, 1)).Range("A:A")) ' find participant gas gate combo

             cons_sum(i, 4) = Application.Sum(Sheets(cons_sum(i, 1)).Range(Cells(cons_sum(i, 3), 5), Cells(cons_sum(i, 3), 16)))

             If cons_sum(i, 4) > 0 Then
                Sheets("Pre-Summary").Cells(i, 4).Value = cons_sum(i, 4)
             End If
        End If
    Next i

    On Error Resume Next
End Sub
glh
  • 4,900
  • 3
  • 23
  • 40