Changing Structure of my Code

[mms_]

Can the code below (in Command1_Click) be re-written so that there is only ONE instance of:
' Print total
Debug.Print " ----"
Debug.Print " " & thisSystemWidthNEW
Debug.Print ""

I think the older I get, the dumber I get, as I think this should be easy.

Also, I would like to get rid of the "total" being printed at the outset,
without using something like If prevSystem <> 0
if possible.

Option Explicit


Private Type SystemFormat
    firstBar As Long
    lastBar As Long
End Type

Dim Bars() As Long
Dim Systems() As SystemFormat


Private Sub Form_Load()
    
    ReDim Systems(1 To 4)
    Systems(1).firstBar = 1:    Systems(1).lastBar = 4
    Systems(2).firstBar = 5:    Systems(2).lastBar = 7
    Systems(3).firstBar = 8:    Systems(3).lastBar = 11
    Systems(4).firstBar = 12:   Systems(4).lastBar = 16
    
    ReDim Bars(1 To 16)
    Bars(1) = 12
    Bars(2) = 22
    Bars(3) = 6
    Bars(4) = 20
    
    Bars(5) = 14
    Bars(6) = 12
    Bars(7) = 34
    
    Bars(8) = 14
    Bars(9) = 30
    Bars(10) = 12
    Bars(11) = 4
    
    Bars(12) = 12
    Bars(13) = 12
    Bars(14) = 12
    Bars(15) = 12
    Bars(16) = 12

End Sub


Private Sub Command1_Click()

    Dim b As Long
    Dim i As Long
    
    Dim thisSystem As Long
    Dim prevSystem As Long
    
    Dim thisBarWidth As Long
    Dim thisSystemWidth As Long

    For b = 1 To UBound(Bars)

        ' Find System Bar is in
        For i = 1 To UBound(Systems)
            If b >= Systems(i).firstBar And b <= Systems(i).lastBar Then
                Exit For
            End If
        Next i
        thisSystem = i
    
        ' System change
        If thisSystem <> prevSystem Then
        
            ' Print total
            Debug.Print "       ----"
            Debug.Print "         " & thisSystemWidth
            Debug.Print ""

            ' Reset
            thisBarWidth = 0
            thisSystemWidth = 0
            prevSystem = thisSystem
            
        End If

        ' Update
        thisBarWidth = Bars(b)
        thisSystemWidth = thisSystemWidth + thisBarWidth
        
        ' Print
        Debug.Print "Bar " & b & " - " & thisBarWidth & "  " & thisSystemWidth
    
    Next b
    
    ' Print LAST total
    Debug.Print "       ----"
    Debug.Print "         " & thisSystemWidth
    Debug.Print ""

End Sub

[SamOscarBrown]

is this the output you want?:

Bar 1 - 12  12
Bar 2 - 22  34
Bar 3 - 6  40
Bar 4 - 20  60
Bar 5 - 14  74
Bar 6 - 12  86
Bar 7 - 34  120
Bar 8 - 14  134
Bar 9 - 30  164
Bar 10 - 12  176
Bar 11 - 4  180
Bar 12 - 12  192
Bar 13 - 12  204
Bar 14 - 12  216
Bar 15 - 12  228
Bar 16 - 12  240
       ----
         240

Because I am not really understanding the issue

[jpbro]

If I've understood correctly, here are 2 possibilities:

You can add a function to print the system width:

Private Sub Command1_Click()
   Dim b As Long
   Dim i As Long

   Dim thisSystem As Long
   Dim prevSystem As Long

   Dim thisBarWidth As Long
   Dim thisSystemWidth As Long

   For b = 1 To UBound(bars)
      ' Find System Bar is in
      For i = 1 To UBound(systems)
         If b >= systems(i).firstBar And b <= systems(i).lastBar Then
            Exit For
         End If
      Next i
      thisSystem = i
   
      ' System change
      If thisSystem <> prevSystem Then
         ' Print total
         PrintTotal thisSystemWidth

         ' Reset
         thisBarWidth = 0
         thisSystemWidth = 0
         prevSystem = thisSystem

      End If

      ' Update
      thisBarWidth = bars(b)
      thisSystemWidth = thisSystemWidth + thisBarWidth

      ' Print
      Debug.Print "Bar " & b & " - " & thisBarWidth & "  " & thisSystemWidth

   Next b
   PrintTotal thisSystemWidth

End Sub

Private Sub PrintTotal(SystemWidth As Long)
   ' Print LAST total
   Debug.Print "       ----"
   Debug.Print "         " & SystemWidth
   Debug.Print ""
End Sub

Or you can add an extra pass to your loop and add logic to skip everything except printing the system width on the last pass:

Private Sub Command1_Click()
   Dim b As Long
   Dim i As Long

   Dim thisSystem As Long
   Dim prevSystem As Long

   Dim thisBarWidth As Long
   Dim thisSystemWidth As Long

   Dim lastPass As Boolean

   For b = 1 To UBound(bars) + 1   ' Add a final pass to the loop
      l_LastPass = (b > UBound(bars))   ' Flag for when we're on the final pass

      ' Find System Bar is in
      If Not l_LastPass Then
         ' Not the final pass, so get system info

         For i = 1 To UBound(systems)
            If b >= systems(i).firstBar And b <= systems(i).lastBar Then
               Exit For
            End If
         Next i
         thisSystem = i
      End If
      
      If (thisSystem <> prevSystem) Or l_LastPass Then
         ' System change or last pass
         ' Print total
         Debug.Print "       ----"
         Debug.Print "         " & thisSystemWidth
         Debug.Print ""

         ' Reset
         thisBarWidth = 0
         thisSystemWidth = 0
         prevSystem = thisSystem
      End If
      
      If Not l_LastPass Then
         ' Not the last pass, so update
         thisBarWidth = bars(b)
         thisSystemWidth = thisSystemWidth + thisBarWidth
   
         ' Print
         Debug.Print "Bar " & b & " - " & thisBarWidth & "  " & thisSystemWidth
      End If
   Next b
End Sub

[SamOscarBrown]

Pretty close (your 2nd example) is what I did...But I am not understanding what OP is wanting for an output.

Will check back later to see, but I've got a few hours of 9-ball and 8-ball coming up in a few minutes in my man-cave! :-)

[mms_]

SamOscarBrown
Output is to be exactly as my original code produces.

Bar 1 - 12  12
Bar 2 - 22  34
Bar 3 - 6  40
Bar 4 - 20  60
       ----
         60

Bar 5 - 14  14
Bar 6 - 12  26
Bar 7 - 34  60
       ----
         60

Bar 8 - 14  14
Bar 9 - 30  44
Bar 10 - 12  56
Bar 11 - 4  60
       ----
         60

Bar 12 - 12  12
Bar 13 - 12  24
Bar 14 - 12  36
Bar 15 - 12  48
Bar 16 - 12  60
       ----
         60

The only problem is (and it's not really a big deal), is that I have to issue the same Print Total code twice
I am just thinking the construct of my For...Next loop is wrong

jpbro
Your 2nd option to "add an extra pass to your loop" does what I was thinking... thanks!
The key is to this extra pass.

ps
If I add
prevSystem = 1
directly before
For b = 1 To UBound(Bars) + 1 ' Add a final pass to the loop
I get rid of the ZERO output at the beginning.

[Schmidt]

IMO you simply have the "nesting" of the two loops backwards
(which leads to all these problems and "extra-flags").

If you loop over your systems in the outer loop (and the bars in the inner),
everything seems to come out quite clean (using only 3 local variables):

Private Sub Command1_Click()
   Dim i As Long, b As Long, grpSum As Double
   
   For i = 1 To UBound(Systems)
       grpSum = 0
       For b = Systems(i).firstBar To Systems(i).lastBar
           grpSum = grpSum + Bars(b)
           Debug.Print "Bar " & b & " - " & Bars(b) & "  " & grpSum
       Next
       Debug.Print "       ----"
       Debug.Print "         " & grpSum
       Debug.Print ""
   Next
End Sub

Olaf

[Niya]

Before we go about trying to refactor this, we need to address a potential bug in OP's original code:-

For b = 1 To UBound(Bars)

        ' Find System Bar is in
        For i = 1 To UBound(Systems)
            If b >= Systems(i).firstBar And b <= Systems(i).lastBar Then
                Exit For
            End If
        Next i

What I highlighted in red doesn't seem to be doing what OP's comment says. He is looking for a bar within a system but the comparison is being done on the For loop's indexing variable b. Should it not be this instead? :-

Bars(b) >= Systems(i).firstBar And Bars(b) <= Systems(i).lastBar

[mms_]

Schmidt
OMG! How incredibly simple!!
Thank you Olaf

Niya
No
The Bars() array variable contains the width of the bar and the loop indexing variable b itself will be the bar number,
so I think it is correct as is.