6
\$\begingroup\$

I have the following code. It's purpose is to add up and sort another columns Data. I tweaked it to fulfil my purpose, but I'm sure it can be cleaned up.

The only problem I have with its function is it leaves a trailing zero if the last item in the referenced list is text.

Can this be changed without altering how it currently reacts to text? Occasionally, I want the zero's if they fall within the middle of the referenced cells, but never if the last cell is text.

My VBA skills are very basic. I can generally alter a piece of code to do what I want, but have no idea on how to fix it.

Public Sub SumCages()
Dim current_row, summary_row, item_total As Integer

current_row = 45
summary_row = 44
While Sheet8.Cells(current_row, 7) <> ""
  If IsNumeric(Sheet8.Cells(current_row, 7)) Then
    item_total = item_total + Val(Sheet8.Cells(current_row, 7))
  Else
    summary_row = summary_row + 1 ' Advance summary_row
    If item_total > 0 Then
      Sheet8.Cells(summary_row, 8) = item_total ' Display total
      current_row = current_row - 1 ' Correct advancement
    Else
      Sheet8.Cells(summary_row, 8) = Sheet8.Cells(current_row, 7) ' Copy label
    End If
    item_total = 0 ' Reset item_total
  End If
  current_row = current_row + 1 ' Advance current_row
Wend
Sheet8.Cells(summary_row + 1, 8) = item_total

current_row = 45
summary_row = 44
While Sheet8.Cells(current_row, 11) <> ""
  If IsNumeric(Sheet8.Cells(current_row, 11)) Then
    item_total = item_total + Val(Sheet8.Cells(current_row, 11))
  Else
    summary_row = summary_row + 1 ' Advance summary_row
    If item_total > 0 Then
      Sheet8.Cells(summary_row, 12) = item_total ' Display total
      current_row = current_row - 1 ' Correct advancement
    Else
      Sheet8.Cells(summary_row, 12) = Sheet8.Cells(current_row, 11) ' Copy label
    End If
    item_total = 0 ' Reset item_total
  End If
  current_row = current_row + 1 ' Advance current_row
Wend
Sheet8.Cells(summary_row + 1, 12) = item_total

current_row = 45
summary_row = 44
While Sheet8.Cells(current_row, 15) <> ""
  If IsNumeric(Sheet8.Cells(current_row, 15)) Then
    item_total = item_total + Val(Sheet8.Cells(current_row, 15))
  Else
    summary_row = summary_row + 1 ' Advance summary_row
    If item_total > 0 Then
      Sheet8.Cells(summary_row, 16) = item_total ' Display total
      current_row = current_row - 1 ' Correct advancement
    Else
      Sheet8.Cells(summary_row, 16) = Sheet8.Cells(current_row, 15) ' Copy label
    End If
    item_total = 0 ' Reset item_total
  End If
  current_row = current_row + 1 ' Advance current_row
Wend
Sheet8.Cells(summary_row + 1, 16) = item_total

current_row = 45
summary_row = 44
While Sheet8.Cells(current_row, 19) <> ""
  If IsNumeric(Sheet8.Cells(current_row, 19)) Then
    item_total = item_total + Val(Sheet8.Cells(current_row, 19))
  Else
    summary_row = summary_row + 1 ' Advance summary_row
    If item_total > 0 Then
      Sheet8.Cells(summary_row, 20) = item_total ' Display total
      current_row = current_row - 1 ' Correct advancement
    Else
      Sheet8.Cells(summary_row, 20) = Sheet8.Cells(current_row, 19) ' Copy label
    End If
    item_total = 0 ' Reset item_total
  End If
  current_row = current_row + 1 ' Advance current_row
Wend
Sheet8.Cells(summary_row + 1, 20) = item_total

current_row = 45
summary_row = 44
While Sheet8.Cells(current_row, 23) <> ""
  If IsNumeric(Sheet8.Cells(current_row, 23)) Then
    item_total = item_total + Val(Sheet8.Cells(current_row, 23))
  Else
    summary_row = summary_row + 1 ' Advance summary_row
    If item_total > 0 Then
      Sheet8.Cells(summary_row, 24) = item_total ' Display total
      current_row = current_row - 1 ' Correct advancement
    Else
      Sheet8.Cells(summary_row, 24) = Sheet8.Cells(current_row, 23) ' Copy label
    End If
    item_total = 0 ' Reset item_total
  End If
  current_row = current_row + 1 ' Advance current_row
Wend
Sheet8.Cells(summary_row + 1, 24) = item_total

current_row = 45
summary_row = 44
While Sheet8.Cells(current_row, 27) <> ""
  If IsNumeric(Sheet8.Cells(current_row, 27)) Then
    item_total = item_total + Val(Sheet8.Cells(current_row, 27))
  Else
    summary_row = summary_row + 1 ' Advance summary_row
    If item_total > 0 Then
      Sheet8.Cells(summary_row, 28) = item_total ' Display total
      current_row = current_row - 1 ' Correct advancement
    Else
      Sheet8.Cells(summary_row, 28) = Sheet8.Cells(current_row, 27) ' Copy label
    End If
    item_total = 0 ' Reset item_total
  End If
  current_row = current_row + 1 ' Advance current_row
Wend
Sheet8.Cells(summary_row + 1, 28) = item_total

current_row = 45
summary_row = 44
While Sheet8.Cells(current_row, 31) <> ""
  If IsNumeric(Sheet8.Cells(current_row, 31)) Then
    item_total = item_total + Val(Sheet8.Cells(current_row, 31))
  Else
    summary_row = summary_row + 1 ' Advance summary_row
    If item_total > 0 Then
      Sheet8.Cells(summary_row, 32) = item_total ' Display total
      current_row = current_row - 1 ' Correct advancement
    Else
      Sheet8.Cells(summary_row, 32) = Sheet8.Cells(current_row, 31) ' Copy label
    End If
    item_total = 0 ' Reset item_total
  End If
  current_row = current_row + 1 ' Advance current_row
Wend
Sheet8.Cells(summary_row + 1, 32) = item_total

current_row = 45
summary_row = 44
While Sheet8.Cells(current_row, 35) <> ""
  If IsNumeric(Sheet8.Cells(current_row, 35)) Then
    item_total = item_total + Val(Sheet8.Cells(current_row, 35))
  Else
    summary_row = summary_row + 1 ' Advance summary_row
    If item_total > 0 Then
      Sheet8.Cells(summary_row, 36) = item_total ' Display total
      current_row = current_row - 1 ' Correct advancement
    Else
      Sheet8.Cells(summary_row, 36) = Sheet8.Cells(current_row, 35) ' Copy label
    End If
    item_total = 0 ' Reset item_total
  End If
  current_row = current_row + 1 ' Advance current_row
Wend
Sheet8.Cells(summary_row + 1, 36) = item_total

current_row = 45
summary_row = 44
While Sheet8.Cells(current_row, 39) <> ""
  If IsNumeric(Sheet8.Cells(current_row, 39)) Then
    item_total = item_total + Val(Sheet8.Cells(current_row, 39))
  Else
    summary_row = summary_row + 1 ' Advance summary_row
    If item_total > 0 Then
      Sheet8.Cells(summary_row, 40) = item_total ' Display total
      current_row = current_row - 1 ' Correct advancement
    Else
      Sheet8.Cells(summary_row, 40) = Sheet8.Cells(current_row, 39) ' Copy label
    End If
    item_total = 0 ' Reset item_total
  End If
  current_row = current_row + 1 ' Advance current_row
Wend
Sheet8.Cells(summary_row + 1, 40) = item_total

current_row = 45
summary_row = 44
While Sheet8.Cells(current_row, 43) <> ""
  If IsNumeric(Sheet8.Cells(current_row, 43)) Then
    item_total = item_total + Val(Sheet8.Cells(current_row, 43))
  Else
    summary_row = summary_row + 1 ' Advance summary_row
    If item_total > 0 Then
      Sheet8.Cells(summary_row, 44) = item_total ' Display total
      current_row = current_row - 1 ' Correct advancement
    Else
      Sheet8.Cells(summary_row, 44) = Sheet8.Cells(current_row, 43) ' Copy label
    End If
    item_total = 0 ' Reset item_total
  End If
  current_row = current_row + 1 ' Advance current_row
Wend
Sheet8.Cells(summary_row + 1, 44) = item_total

current_row = 45
summary_row = 44
While Sheet8.Cells(current_row, 47) <> ""
  If IsNumeric(Sheet8.Cells(current_row, 47)) Then
    item_total = item_total + Val(Sheet8.Cells(current_row, 47))
  Else
    summary_row = summary_row + 1 ' Advance summary_row
    If item_total > 0 Then
      Sheet8.Cells(summary_row, 48) = item_total ' Display total
      current_row = current_row - 1 ' Correct advancement
    Else
      Sheet8.Cells(summary_row, 48) = Sheet8.Cells(current_row, 47) ' Copy label
    End If
    item_total = 0 ' Reset item_total
  End If
  current_row = current_row + 1 ' Advance current_row
Wend
Sheet8.Cells(summary_row + 1, 48) = item_total

current_row = 45
summary_row = 44
While Sheet8.Cells(current_row, 51) <> ""
  If IsNumeric(Sheet8.Cells(current_row, 51)) Then
    item_total = item_total + Val(Sheet8.Cells(current_row, 51))
  Else
    summary_row = summary_row + 1 ' Advance summary_row
    If item_total > 0 Then
      Sheet8.Cells(summary_row, 52) = item_total ' Display total
      current_row = current_row - 1 ' Correct advancement
    Else
      Sheet8.Cells(summary_row, 52) = Sheet8.Cells(current_row, 51) ' Copy label
    End If
    item_total = 0 ' Reset item_total
  End If
  current_row = current_row + 1 ' Advance current_row
Wend
Sheet8.Cells(summary_row + 1, 52) = item_total

current_row = 45
summary_row = 44
While Sheet8.Cells(current_row, 55) <> ""
  If IsNumeric(Sheet8.Cells(current_row, 55)) Then
    item_total = item_total + Val(Sheet8.Cells(current_row, 55))
  Else
    summary_row = summary_row + 1 ' Advance summary_row
    If item_total > 0 Then
      Sheet8.Cells(summary_row, 56) = item_total ' Display total
      current_row = current_row - 1 ' Correct advancement
    Else
      Sheet8.Cells(summary_row, 56) = Sheet8.Cells(current_row, 55) ' Copy label
    End If
    item_total = 0 ' Reset item_total
  End If
  current_row = current_row + 1 ' Advance current_row
Wend
Sheet8.Cells(summary_row + 1, 56) = item_total

current_row = 45
summary_row = 44
While Sheet8.Cells(current_row, 59) <> ""
  If IsNumeric(Sheet8.Cells(current_row, 59)) Then
    item_total = item_total + Val(Sheet8.Cells(current_row, 59))
  Else
    summary_row = summary_row + 1 ' Advance summary_row
    If item_total > 0 Then
      Sheet8.Cells(summary_row, 60) = item_total ' Display total
      current_row = current_row - 1 ' Correct advancement
    Else
      Sheet8.Cells(summary_row, 60) = Sheet8.Cells(current_row, 59) ' Copy label
    End If
    item_total = 0 ' Reset item_total
  End If
  current_row = current_row + 1 ' Advance current_row
Wend
Sheet8.Cells(summary_row + 1, 60) = item_total

current_row = 45
summary_row = 44
While Sheet8.Cells(current_row, 63) <> ""
  If IsNumeric(Sheet8.Cells(current_row, 63)) Then
    item_total = item_total + Val(Sheet8.Cells(current_row, 63))
  Else
    summary_row = summary_row + 1 ' Advance summary_row
    If item_total > 0 Then
      Sheet8.Cells(summary_row, 64) = item_total ' Display total
      current_row = current_row - 1 ' Correct advancement
    Else
      Sheet8.Cells(summary_row, 64) = Sheet8.Cells(current_row, 63) ' Copy label
    End If
    item_total = 0 ' Reset item_total
  End If
  current_row = current_row + 1 ' Advance current_row
Wend
Sheet8.Cells(summary_row + 1, 64) = item_total

current_row = 45
summary_row = 44
While Sheet8.Cells(current_row, 67) <> ""
  If IsNumeric(Sheet8.Cells(current_row, 67)) Then
    item_total = item_total + Val(Sheet8.Cells(current_row, 67))
  Else
    summary_row = summary_row + 1 ' Advance summary_row
    If item_total > 0 Then
      Sheet8.Cells(summary_row, 68) = item_total ' Display total
      current_row = current_row - 1 ' Correct advancement
    Else
      Sheet8.Cells(summary_row, 68) = Sheet8.Cells(current_row, 67) ' Copy label
    End If
    item_total = 0 ' Reset item_total
  End If
  current_row = current_row + 1 ' Advance current_row
Wend
Sheet8.Cells(summary_row + 1, 68) = item_total

current_row = 45
summary_row = 44
While Sheet8.Cells(current_row, 71) <> ""
  If IsNumeric(Sheet8.Cells(current_row, 71)) Then
    item_total = item_total + Val(Sheet8.Cells(current_row, 71))
  Else
    summary_row = summary_row + 1 ' Advance summary_row
    If item_total > 0 Then
      Sheet8.Cells(summary_row, 72) = item_total ' Display total
      current_row = current_row - 1 ' Correct advancement
    Else
      Sheet8.Cells(summary_row, 72) = Sheet8.Cells(current_row, 71) ' Copy label
    End If
    item_total = 0 ' Reset item_total
  End If
  current_row = current_row + 1 ' Advance current_row
Wend
Sheet8.Cells(summary_row + 1, 72) = item_total

current_row = 45
summary_row = 44
While Sheet8.Cells(current_row, 75) <> ""
  If IsNumeric(Sheet8.Cells(current_row, 75)) Then
    item_total = item_total + Val(Sheet8.Cells(current_row, 75))
  Else
    summary_row = summary_row + 1 ' Advance summary_row
    If item_total > 0 Then
      Sheet8.Cells(summary_row, 76) = item_total ' Display total
      current_row = current_row - 1 ' Correct advancement
    Else
      Sheet8.Cells(summary_row, 76) = Sheet8.Cells(current_row, 75) ' Copy label
    End If
    item_total = 0 ' Reset item_total
  End If
  current_row = current_row + 1 ' Advance current_row
Wend
Sheet8.Cells(summary_row + 1, 76) = item_total

current_row = 45
summary_row = 44
While Sheet8.Cells(current_row, 79) <> ""
  If IsNumeric(Sheet8.Cells(current_row, 79)) Then
    item_total = item_total + Val(Sheet8.Cells(current_row, 79))
  Else
    summary_row = summary_row + 1 ' Advance summary_row
    If item_total > 0 Then
      Sheet8.Cells(summary_row, 80) = item_total ' Display total
      current_row = current_row - 1 ' Correct advancement
    Else
      Sheet8.Cells(summary_row, 80) = Sheet8.Cells(current_row, 79) ' Copy label
    End If
    item_total = 0 ' Reset item_total
  End If
  current_row = current_row + 1 ' Advance current_row
Wend
Sheet8.Cells(summary_row + 1, 80) = item_total

current_row = 45
summary_row = 44
While Sheet8.Cells(current_row, 83) <> ""
  If IsNumeric(Sheet8.Cells(current_row, 83)) Then
    item_total = item_total + Val(Sheet8.Cells(current_row, 83))
  Else
    summary_row = summary_row + 1 ' Advance summary_row
    If item_total > 0 Then
      Sheet8.Cells(summary_row, 84) = item_total ' Display total
      current_row = current_row - 1 ' Correct advancement
    Else
      Sheet8.Cells(summary_row, 84) = Sheet8.Cells(current_row, 83) ' Copy label
    End If
    item_total = 0 ' Reset item_total
  End If
  current_row = current_row + 1 ' Advance current_row
Wend
Sheet8.Cells(summary_row + 1, 84) = item_total

current_row = 45
summary_row = 44
While Sheet8.Cells(current_row, 87) <> ""
  If IsNumeric(Sheet8.Cells(current_row, 87)) Then
    item_total = item_total + Val(Sheet8.Cells(current_row, 87))
  Else
    summary_row = summary_row + 1 ' Advance summary_row
    If item_total > 0 Then
      Sheet8.Cells(summary_row, 88) = item_total ' Display total
      current_row = current_row - 1 ' Correct advancement
    Else
      Sheet8.Cells(summary_row, 88) = Sheet8.Cells(current_row, 87) ' Copy label
    End If
    item_total = 0 ' Reset item_total
  End If
  current_row = current_row + 1 ' Advance current_row
Wend
Sheet8.Cells(summary_row + 1, 88) = item_total

End Sub
\$\endgroup\$
2
  • \$\begingroup\$ At first glance this looks very repetitive; I'll take a deeper look a bit later, but for now I'm pretty sure you can write a function that starts with current_row = 45 and ends with Sheet8.Cells(summary_row + 1, column) = item_total, and call it as many times as you're repeating this code block. Copy+Paste is never a good sign when coding... \$\endgroup\$
    – Mathieu Guindon
    Commented Sep 10, 2013 at 22:37
  • \$\begingroup\$ How and where is the sorting taking place? \$\endgroup\$
    – Mathieu Guindon
    Commented Sep 11, 2013 at 0:24

1 Answer 1

Reset to default
4
\$\begingroup\$

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.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.