Skip to main content
2 of 2
added 1684 characters in body
Mathieu Guindon
  • 75.1k
  • 18
  • 191
  • 463

There are many iterative structures in VBA, but I think the one that would be most useful here is For...Next...Step; in your case, looks like Step should be 4, looping from 7 to 87.

All of your code blocks look very, very similar. I would only keep a single block and replace the hard-coded "magic numbers" with variables.

Something like this:

Private Const SUMMARY_ROW As Integer = 44
Private Const START_ROW As Integer = 45 'these two never change.
'...and if they ever do, that's the only place you need to change them.

Public Sub SumCages()
    Dim c As Integer
    For c = 7 To 87 Step 4 'start at 7 and +4 per iteration, up to 87
        CalculateTotal c
    Next
End Sub

The CalculateTotal procedure is nothing more than one of your code blocks, with the hard-coded row numbers replaced by the SumColumn parameter, which comes from the loop above.

Notice variables' names; variables are your friends, whenever you find yourself repeating a given operation (like, Sheet8.Cells(current_row + 1, SumColumn)), you can improve readability by adding a variable - this way at a glance you know two different places are talking about the same thing.

Private Sub CalculateTotal(sumColumn As Integer)
    
    Dim currentRow  As Integer
    Dim targetRow   As Integer
    Dim columnTotal As Integer
    Dim thisValue   As String
    
    currentRow = START_ROW
    targetRow = SUMMARY_ROW
    targetColumn = sumColumn + 1
    
    With Sheet8 ' this avoids having to specify it every time
        
        thisValue = .Cells(currentRow, sumColumn)
        
        While thisValue <> vbNullString

            If IsNumeric(thisValue) Then
                
                'this is where the sum actually takes place:
                columnTotal = columnTotal + Val(thisValue)
                
            Else
                If columnTotal > 0 Then
                   
                    .Cells(targetRow + 1, targetColumn) = columnTotal
                    
                Else

                    targetRow = targetRow + 1 'not sure I get why this is done...
                    currentRow = currentRow - 1 '...and this is even more confusing...
                    .Cells(targetRow, targetColumn) = thisValue
                    
                End If
                
                'this is likely throwing off your total!
                columnTotal = 0
            End If
            
            currentRow = currentRow + 1
            thisValue = .Cells(currentRow, SumColumn)
        Wend

        .Cells(targetRow, targetColumn) = columnTotal
        
    End With
End Sub

It would be nice if you could post a screenshot of what the result looks like, because it's quite confusing (to me, at least) why you're incrementing your write-to row and then decrementing your loop counter, but only when you hit a non-numeric value.

To me it looks like what you have as current_row = current_row - 1 ' Correct advancement should really read summary_row = summary_row - 1 ' Correct advancement, because I would think it's an infinite loop written like this.

Mathieu Guindon
  • 75.1k
  • 18
  • 191
  • 463