12
\$\begingroup\$

Being new to VBA, and many years removed from college programming courses in Java, I'm seeking opinions about design, implementation, readability and general better practices using VBA to tackle complex problems. There are probably other programming languages much better suited to this problem, but I'm limited by the resources available (excel only) to the users it's intended for.

I'm developing a program to forecast maintenance intervals for a fleet of aircraft, by week. The aircraft are maintained in a phased cycle based on hours flown. The goal of the program is to determine, by week, when each aircraft will be due for it's next maintenance cycle, if in any week the fleet of aircraft cannot fly all the planned hours and when, if at all, two aircraft run out of hours and end up requiring maintenance in the same week, given the following constraints.

I currently spend an inordinate amount of time fat-fingering a spreadsheet to do precisely this, and the inter-related nature of each aircraft's hours flown in a given week, and their maintenance cycles seems uniquely suited for a computer program to figure out.

Constraints:

  1. Each aircraft is required to enter it's maintenance cycle after a fix amount of hours flown (e.g. 200)
  2. Only one aircraft can be in a maintenance cycle at once
  3. Each maintenance cycle requires differing amounts of time to complete (e.g. after 200 hours, the cycle takes one week to complete, but after 800 hours it takes five weeks)
  4. Each week the fleet of aircraft will have a fix number of hours to fly
  5. Each aircraft may be assigned to fly a fixed number of hours in a given week

The Goal

The end goal of the program is to determine both when you run out of hours and two aircraft are forecast to be in a maintenance cycle in the same week, or when you can't meet the planned hours for the fleet in a given week (e.g.run out of hours) by not allowing more than one aircraft to be in a cycle at the same time.

Inputs:

An Excel Workbook with two Worksheets, "Matrix" for the displaying the results of the program and "Matrix Inputs" containing most of the initial data (some is on the "Matrix" sheet). I made use of global named ranges as much as possible, and all inputs from and outputs to are based on relative references to these ranges.

Matrix Worksheet

Matrix

Matrix Inputs Worksheet

Matrix Inputs

Maintenance Cycles

Aircraft are due for maintenance periodically based on the amount an aircraft is flown, and each cycle takes a differing amount of time to complete.

Cycle ID    Cycle Types    Duration in weeks    Text ID
1           200 hour           1                HMT1
2           200/400 hour       2                HMT2
3           200/600 hour       1                HMT3
...
12          200/400/800 hour   5                HMT12

An Aircraft

I began by creating an aircraft object, with properties capturing initial state and current state. Only current state properties are changeable. I realize now that setting and maintaining initial state properties may not be necessary and adds to the complexity, as I can just read/set the initial properties from the spreadsheet again if necessary.

An aircraft has six known properties, which are set by reading values from a worksheet when an object instance is initialized.

  1. Tail Number (aka unique id)
  2. Current aircraft Hours
  3. Next cycle due (type)
  4. Aircraft hours when the next maintenance cycle is due
  5. Is the aircraft currently in a maintenance cycle (Boolean)
  6. The week the next cycle will start (set by calculation)

Class c_Aircraft

Option Explicit

'Class c_Aircraft: _
Requires call to init_aircraft() with the Range and "row" of the aircraft instance to be created. _
Assumes the Range contains the following columns: _
1            2                3            4                5            6 _
Tail Number  Current Hours    Next Cycle   Next Cycle DUE   In Heavy     Week Id Cycle Start _
1104         11265.0          4            11333.7          FALSE        [Integer or -blank if not in heavy]


'Design of class based upon need to implement ability to copy an aircraft object
'http://stackoverflow.com/questions/4805475/assignment-of-objects-in-vb6/4805812#4805812
Private Type Aircraft
      p_tailNumber As Integer '*
      p_tailNumStr As String

      p_initialAircraftHours As Double '*
      p_currentAircraftHours As Double

      p_initialNextCycleType As Integer '*
      p_nextCycleType As Integer

      p_initialNextCycleDuration As Integer '*
      p_nextCycleDuration As Integer

      p_initialHoursNextCycleDue As Double '*
      p_hoursNextCycleDue As Double

      p_initialInHeavy As Boolean '*
      p_inHeavy As Boolean

      'An integer representing the Week Id the current cycle started, if in heavy maintenance
      p_initialWeekIdCycleStart As Integer
      p_weekIdCycleStart As Integer

      p_initialHoursToDUE As Double
      p_hoursToDUE As Double

      p_initialHoursToDNE As Double
      p_hoursToDNE As Double
End Type

Private acft As Aircraft

'General Methods

Public Sub weeklyUpdate(ByVal hoursFlown As Double)
      'Update current aircraft hours
      acft.p_currentAircraftHours = acft.p_currentAircraftHours - hoursFlown
      'Update hoursto the DUE
      acft.p_hoursToDUE = acft.p_hoursToDUE - hoursFlown
      'Update hours to the DNE
      acft.p_hoursToDNE = acft.p_hoursToDNE - hoursFlown
End Sub

'Update this instance of Object Aircraft when entering of leaving a heavy mx cycle
Public Sub setInHeavy(ByVal inHeavy As Boolean, ByVal weekIdCycleStarted As Integer, mxCycles() As Variant)
      If Not inHeavy Then
            acft.p_inHeavy = inHeavy
            acft.p_weekIdCycleStart = -10
            'Set next cycle type
            If (acft.p_nextCycleType + 1 > 12) Then
                  acft.p_nextCycleType = 1
            Else
                  acft.p_nextCycleType = acft.p_nextCycleType + 1
            End If

            'duration
            acft.p_nextCycleDuration = mxCycles(acft.p_nextCycleType, 2)

            'Aircraft hours at the next cycle due
            acft.p_hoursNextCycleDue = acft.p_currentAircraftHours + 200
            'Hours remaining to DUE
            acft.p_hoursToDUE = 200

            'Hours remaining to DUE
            acft.p_hoursToDNE = 215

      Else
            acft.p_weekIdCycleStart = weekIdCycleStarted
      End If
End Sub

'Custom Initialize
Public Sub init_aircraft(data_range As Range, ByVal asset_number As Integer, mxCycles() As Variant)
      acft.p_tailNumber = CInt(data_range(asset_number, 1))
      acft.p_tailNumStr = CStr(acft.p_tailNumber)
      acft.p_initialAircraftHours = CDbl(data_range(asset_number, 2))
      acft.p_currentAircraftHours = p_initialAcftHours
      acft.p_initialNextCycleType = data_range(asset_number, 3)
      acft.p_nextCycleType = p_initialNextCycleType
      acft.p_initialNextCycleDuration = mxCycles(acft.p_nextCycleType, 2)
      acft.p_nextCycleDuration = p_initialNextCycleDuration
      acft.p_initialHoursNextCycleDue = data_range(asset_number, 4)
      acft.p_hoursNextCycleDue = p_initialHoursNextCycleDue
      acft.p_initialInHeavy = data_range(asset_number, 5)
      acft.p_inHeavy = p_initialInHeavy

      If acft.p_inHeavy Then
            acft.p_initialWeekIdCycleStart = data_range(asset_number, 6)
            acft.p_weekIdCycleStart = p_initialWeekIdCycleStart
      Else
        'set to a week prior more than the longest cycle duration
        acft.p_initialWeekIdCycleStart = -10
        acft.p_weekIdCycleStart = -10
      End If

      acft.p_initialHoursToDUE = Round(acft.p_hoursNextCycleDue - acft.p_currentAircraftHours, 1)
      acft.p_hoursToDUE = p_initialHoursToDUE
      acft.p_initialHoursToDNE = Round(acft.p_hoursNextCycleDue - acft.p_currentAircraftHours + 15, 1)
      acft.p_hoursToDNE = p_initialHoursToDNE
End Sub

'Return the aircraft objects properties as String
Public Function print_aircraft() As String
    print_aircraft = acft.p_tailNumber & vbCrLf & _
                        "Current Hours: " & acft.p_currentAircraftHours & vbCrLf & _
                        "Next Cycle: " & acft.p_nextCycleType & vbCrLf & _
                        "Next Cycle Duration: " & acft.p_nextCycleDuration & vbCrLf & _
                        "Hours Next Cycle Due: " & acft.p_hoursNextCycleDue & vbCrLf & _
                        "In Heavy: " & acft.p_inHeavy & vbCrLf & _
                        "Week Id Cycle Start: " & acft.p_weekIdCycleStart & vbCrLf & _
                        "DUE: " & acft.p_hoursToDUE & vbCrLf & _
                        "DNE: " & acft.p_hoursToDNE
End Function

'Get/Let Methods

' Hours Remaining to the DNE
Public Property Get hoursToDNE() As Double
    hoursToDNE = acft.p_hoursToDNE
End Property

Public Property Let hoursToDNE(ByVal HoursDNE As Double)
      acft.p_hoursToDNE = HoursDNE
End Property

Private Property Get p_initialHoursToDNE() As Double
      p_initialHoursToDNE = acft.p_initialHoursToDNE
End Property

' Hours Remaining to the DUE
Public Property Get hoursToDUE() As Double
    hoursToDUE = acft.p_hoursToDUE
End Property

Public Property Let hoursToDUE(ByVal HoursDUE As Double)
      acft.p_hoursToDUE = HoursDUE
End Property

Private Property Get p_initialHoursToDUE() As Double
      p_initialHoursToDUE = acft.p_initialHoursToDUE
End Property

'Week ID of the next heavy cycle start
Public Property Get weekIdCycleStart() As Integer
    weekIdCycleStart = acft.p_weekIdCycleStart
End Property

Public Property Let weekIdCycleStart(ByVal weekId As Integer)
    acft.p_weekIdCycleStart = weekId
End Property

Private Property Get p_initialWeekIdCycleStart() As Integer
      p_initialWeekIdCycleStart = acft.p_initialWeekIdCycleStart
End Property

' Aircraft in Heavy Property
Public Property Get inHeavy() As Boolean
    inHeavy = acft.p_inHeavy
End Property

Private Property Get p_initialInHeavy() As Integer
      p_initialInHeavy = acft.p_initialInHeavy
End Property

' Aircraft Hours at Next Maintenance Cycle Due Property
Public Property Get hoursNextCycleDue() As Double
      hoursNextCycleDue = acft.p_hoursNextCycleDue
End Property

Public Property Let hoursNextCycleDue(ByVal Value As Double)
      acft.p_hoursNextCycleDue = Value
End Property

Private Property Get p_initialHoursNextCycleDue() As Double
      p_initialHoursNextCycleDue = acft.p_initialHoursNextCycleDue
End Property

' Next Maintenance Cycle Duration Property
Public Property Get nextCycleDuration() As Integer
      nextCycleDuration = acft.p_nextCycleDuration
End Property

Public Property Let nextCycleDuration(ByVal cycleDuration As Integer)
      acft.p_nextCycleDuration = cycleDuration
End Property

Private Property Get p_initialNextCycleDuration() As Integer
      p_initialNextCycleDuration = acft.p_initialNextCycleDuration
End Property

' Next Maintenance Cycle Due Property
Public Property Get nextCycleType() As Integer
      nextCycleType = acft.p_nextCycleType
End Property

Public Property Let nextCycleType(ByVal cycleType As Integer)
      acft.p_nextCycleType = cycleType
End Property

Private Property Get p_initialNextCycleType() As Integer
      p_initialNextCycleType = acft.p_initialNextCycleType
End Property

' Current Aircraft Hours Property
Public Property Get currentAircraftHours() As Double
      currentAircraftHours = acft.p_currentAircraftHours
End Property

Public Property Let currentAircraftHours(ByVal Value As Double)
      acft.p_currentAircraftHours = Value
End Property

Private Property Get p_initialAcftHours() As Double
      p_initialAcftHours = acft.p_initialAircraftHours
End Property

' Tail Number Property
Public Property Get tailNumber() As Integer
      tailNumber = acft.p_tailNumber
End Property

Public Property Get tailNumStr() As String
      tailNumStr = acft.p_tailNumStr
End Property

Public Property Let tailNumber(ByVal Value As Integer)
      acft.p_tailNumber = Value
      acft.p_tailNumStr = CStr(Value)
End Property

Private Property Get p_tailNbr() As Integer
      p_tailNbr = acft.p_tailNumber
End Property

'memento function
Friend Sub SetAcftMemento(NewAcftMemento As Aircraft)
      acft = NewAcftMemento
End Sub

'Return a new c_Aircraft object which is a duplicate of this instance of c_Aircraft
Public Function Copy() As c_Aircraft
      Dim Result As c_Aircraft
      Set Result = New c_Aircraft
      Call Result.SetAcftMemento(acft)
      Set Copy = Result
End Function

I start by reading in all the data from the Matrix Inputs sheet and setting up the various Arrays and Collections. The real work is done in the sortedAircraft function, and the calculateWeek sub.

The order of business

  1. Create a Collection of c_Aircraft instances and set their values from the input worksheet
  2. Sort the aircraft ascending based on remaining hours due to the next maintenance cycle, beginning with the aircraft currently in a cycle, if any.
  3. Determine the start week of each aircraft's next cycle based on the projected end week of the previous aircraft, with one week gap in between
  4. Calculate the hours flown for all aircraft in a given week

Step 4 is the key since there are three constraints to setting an individual aircraft's hours to fly in a given week: the planned total hours for the fleet, the need to ensure an individual aircraft has enough hours to spread among the remaining weeks until it enters it's next cycle, and any individual aircraft's fixed hours to fly that week.

I realized yesterday (after the original post) when I started inputting different data that the algorithm doesn't work completely as intended

Here's where the magic happens:

  Option Explicit
  Const acft_range_row_offset As Integer = 3
  Const remaining_col_offset As Integer = 2
  Const weekly_hours_col_offset As Integer = 1
  Const cycle_txt_id_col As Integer = 3
  Const deploy_hours_col As Integer = 2
  Const notInHeavyCycleStart As Integer = -10
  Const viewportOffset As Integer = 10
  Const minHoursPerWeek As Double = 10
  Dim mx_cycles() As Variant
  Dim calculateToDNE As Boolean


  'Dim deployment_hours() As Variant

  Sub calculateMaintenancePlan()
          'Calculates next x maintenance cycles for a number aircraft _

          'clear the spreasheet
          clearMxCycles False

          'debugging/utility variables
          Static counter As Long
          Dim temp As Variant
          Dim i As Integer, j As Integer, startWeek As Integer, endWeek As Integer

          'Read in the flag to determine whether to base calculations on the hours to DUE or hours to DNE (Do Not Exceed)
          calculateToDNE = Worksheets("Matrix Inputs").Range("maintenance_cycles").Cells(1, 3)

          'Assumes the spreadsheet defines a named range containing the following columns:
          'Cycle ID     Cycle Type      Duration    Text ID
          '1            200 hour        1           HMT1
          ReDim mx_cycles(1 To Worksheets("Matrix Inputs").Range("maintenance_cycles").Rows.Count, 1 To 3)
          For i = 1 To Worksheets("Matrix Inputs").Range("maintenance_cycles").Rows.Count
                    mx_cycles(i, 1) = Worksheets("Matrix Inputs").Range("maintenance_cycles").Cells(i, 2)
                    mx_cycles(i, 2) = CInt(Worksheets("Matrix Inputs").Range("maintenance_cycles").Cells(i, 3))
                    mx_cycles(i, 3) = Worksheets("Matrix Inputs").Range("maintenance_cycles").Cells(i, 4)
                    'Debug.Print i & " : " & mx_cycles(i, 1) & " : " & mx_cycles(i, 2) & " : " & mx_cycles(i, 3)
          Next i

          'Collection containing each aircraft tail number, assume spreadsheet
          'contains named range called "aircraft"
          Dim Aircraft As New Collection, s_Aircraft As New Collection
          Dim acft As c_Aircraft

          'Set the Collection size to the total number of aircraft on station
          'and create a c_Aircraft instance representing each airframe
          For i = 1 To Range("aircraft").Count
                    Set acft = New c_Aircraft
                    acft.init_aircraft Worksheets("Matrix Inputs").Range("inputs"), i, mx_cycles
                    'Debug.Print acft.print_aircraft
                    Aircraft.Add acft, acft.tailNumStr
          Next i

          'debug - limit the calculation
          startWeek = 1
          endWeek = 22

          'The real work is done here:
          For i = startWeek To endWeek
                    'Sort the aircraft (heavy mx plane to most hours remaining)
                    Set s_Aircraft = sortedAircraft(Aircraft)
                    calculateWeek s_Aircraft, Aircraft, i
          Next i

                    If cellIsInVisibleRange(Range("week_id").Cells(1, WorksheetFunction.Match(endWeek, Range("week_id"), 0))) Then
                    If WorksheetFunction.Match(endWeek, Range("week_id"), 0) < viewportOffset Then
                            Application.Goto Range("week_id").Cells(1, 1), True
                    Else
                            Application.Goto Range("week_id").Cells(1, WorksheetFunction.Match(endWeek, Range("week_id"), 0) - viewportOffset), True
                    End If
          End If

          counter = counter + 1
          Debug.Print "End run #" & counter & vbCrLf

  End Sub

  'Sort a Collection of c_aircraft objects from aircraft in heavy maintenance cycle descending to the high time aircraft, _
  and set the week id the earliest each aircraft's next cycle could start
  Private Function sortedAircraft(unSortedAircraft As Collection) As Collection
          Set sortedAircraft = New Collection
          Dim ac As New c_Aircraft, t_acft As New c_Aircraft
          Dim i As Long, j As Long
          Dim next_acft_cycle_start_week_id As Integer
          Dim previous_acft As String

          'copy the Collection to a new collection
          For Each ac In unSortedAircraft
                    Set t_acft = ac.Copy
                    sortedAircraft.Add t_acft, t_acft.tailNumStr
          Next ac

          'Sort the aircraft from in heavy/lowest to highest hours to DUE
          For i = 1 To sortedAircraft.Count
                For j = i + 1 To sortedAircraft.Count
                      If sortedAircraft.Item(i).hoursToDUE > sortedAircraft.Item(j).hoursToDUE Then
                            Set t_acft = sortedAircraft.Item(j).Copy
                            sortedAircraft.Remove j
                            sortedAircraft.Add t_acft, t_acft.tailNumStr, i
                      End If
                Next j
          Next i

          'Update the week id the earliest each aircraft's next cycle could start, e.g. 6039 starts on week -1, runs for five weeks (weeks -1, 0 , 1, 2 and 3), _
          add 1 week between cycles, and the 6005 starts on week 5
          previous_acft = sortedAircraft.Item(1).tailNumStr

          For Each ac In sortedAircraft
                    If ac.inHeavy = False Then
                            ac.weekIdCycleStart = sortedAircraft.Item(previous_acft).weekIdCycleStart + sortedAircraft.Item(previous_acft).nextCycleDuration + 1
                            'Debug.Print vbCrLf & "********" & vbCrLf & ac.tailNumber & ".weekIdCycleStart: " & ac.weekIdCycleStart
                            previous_acft = ac.tailNumStr
                    Else
                            'Debug.Print vbCrLf & "********" & vbCrLf & ac.tailNumber & ".weekIdCycleStart: " & ac.weekIdCycleStart
                            previous_acft = ac.tailNumStr
                    End If
          Next ac

  End Function

  'calculate one week of aircraft hours/maintenance cycles
  Private Sub calculateWeek(sortedAircraft As Collection, unSortedAircraft As Collection, currentWeekId As Integer)
          Dim numInHeavy As Integer
          Dim acft As c_Aircraft
          Dim hrs_this_wk As Double, hrs_div_wks As Double
          Dim rowIndex As Long
          Dim next_acft_cycle_start_week_id As Integer, i As Integer
          'variables preceeded by "r_" or "c_" represent row or column indexes as implied
          Dim r_hrs_per_wk As Integer, c_current_wk As Integer, r_deploy_type As Integer
          Dim deploy_type As String
          Dim weekly_hrs As Double, hoursFlown As Double, acft_hrs_remaining As Double, wks_remaining As Double

          c_current_wk = WorksheetFunction.Match(currentWeekId, Range("week_id"), 0)
          weekly_hrs = Range("planned_hours").Cells(1, c_current_wk)
          hoursFlown = 0

          Select Case numberInHeavy(sortedAircraft)

                    Case Is <= 1 'One or less plane in heavy

                            'Is the plane in heavy?
                            For i = sortedAircraft.Count To 1 Step -1
                                      Set acft = sortedAircraft.Item(i)
                                      r_hrs_per_wk = WorksheetFunction.Match(acft.tailNumber, Range("aircraft"), 0)
                                      c_current_wk = WorksheetFunction.Match(currentWeekId, Range("week_id"), 0)

                                      If acft.inHeavy Then
                                              'Set the 'Hours/Week' cell to the appropriate number if weekly flight hours
                                              Range("mx_plan").Cells(r_hrs_per_wk, c_current_wk + weekly_hours_col_offset) = hoursFlown
                                              'Debug.Print mx_cycles(sortedAircraft.Item(i).nextCycleType, 3) & " - " & sortedAircraft.Item(i).nextCycleDuration + (sortedAircraft.Item(i).weekIdCycleStart - currentWeekId - 1) & " weeks remaining"

                                              'Set the 'Remaining' cell to the appropriate text id for the current heavy maintenance cycle
                                              Range("mx_plan").Cells(r_hrs_per_wk, c_current_wk + remaining_col_offset) = mx_cycles(acft.nextCycleType, cycle_txt_id_col)

                                              'If heavy cycles complete, update acft hours, and set acft inHeavy to false, set nextCycleType and weekIdCycleStart properties to reflect
                                              If acft.nextCycleDuration + (acft.weekIdCycleStart - currentWeekId - 1) = 0 Then
                                                        'Update acft hours for completion of mx cycle
                                                        unSortedAircraft.Item(acft.tailNumStr).setInHeavy False, currentWeekId + 1, mx_cycles
                                                        Debug.Print "HEAVY Complete"
                                              End If
                                              sortedAircraft.Remove (i)
                                      End If
                            Next i

                            'Is the plane deployed or does it have fixed hours this week?
                            For i = sortedAircraft.Count To 1 Step -1
                                      Set acft = sortedAircraft.Item(i)
                                      r_hrs_per_wk = WorksheetFunction.Match(acft.tailNumber, Range("aircraft"), 0)
                                      c_current_wk = WorksheetFunction.Match(currentWeekId, Range("week_id"), 0)
                                      deploy_type = deploymentType(acft, currentWeekId)

                                      If deploy_type <> "" Then
                                              r_deploy_type = WorksheetFunction.Match(deploy_type, Worksheets("Matrix Inputs").Range("deployment_list"), 0)
                                              hrs_this_wk = Worksheets("Matrix Inputs").Range("deployments").Cells(r_deploy_type, deploy_hours_col)

                                              'Set the 'Hours/Week' cell to the appropriate number if weekly flight hours
                                              Range("mx_plan").Cells(r_hrs_per_wk, c_current_wk + weekly_hours_col_offset) = hrs_this_wk

                                              'Subtract the current aircraft's weekly hours from the total planned hours remaining
                                              weekly_hrs = weekly_hrs - hrs_this_wk
                                              'Set the 'Remaining' cell to the hours remaining to the next cycle DUE/DNE, depending on the calculateToDNE flag
                                              If calculateToDNE Then
                                                        Range("mx_plan").Cells(r_hrs_per_wk, c_current_wk + remaining_col_offset) = acft.hoursToDNE - hrs_this_wk
                                              Else
                                                        Range("mx_plan").Cells(r_hrs_per_wk, c_current_wk + remaining_col_offset) = acft.hoursToDUE - hrs_this_wk
                                              End If

                                              'Update acft instance for completion of week
                                              unSortedAircraft.Item(acft.tailNumStr).weeklyUpdate (hrs_this_wk)
                                              sortedAircraft.Remove (i)
                                      End If
                            Next i

                            'Is the plane low time? _
                            Low Time = less than XX hours per week over the remaining weeks until the next cycle.  XX = 10 hours, for now
                            For i = sortedAircraft.Count To 1 Step -1
                                      Set acft = sortedAircraft.Item(i)
                                      r_hrs_per_wk = WorksheetFunction.Match(acft.tailNumber, Range("aircraft"), 0)
                                      c_current_wk = WorksheetFunction.Match(currentWeekId, Range("week_id"), 0)

                                      If calculateToDNE Then
                                              acft_hrs_remaining = acft.hoursToDNE
                                      Else
                                              acft_hrs_remaining = acft.hoursToDUE
                                      End If
                                      wks_remaining = acft.weekIdCycleStart - currentWeekId

                                      hrs_this_wk = Round(acft_hrs_remaining / wks_remaining, 1)

                                      If hrs_this_wk < 10 Then
                                              Range("mx_plan").Cells(r_hrs_per_wk, c_current_wk + weekly_hours_col_offset) = hrs_this_wk
                                              Range("mx_plan").Cells(r_hrs_per_wk, c_current_wk + remaining_col_offset) = acft_hrs_remaining - hrs_this_wk

                                              weekly_hrs = weekly_hrs - hrs_this_wk

                                              unSortedAircraft.Item(acft.tailNumStr).weeklyUpdate (hrs_this_wk)
                                              sortedAircraft.Remove (i)
                                      End If
                            Next i

                            'Divide up the remaining planned weekly hours amoungst the remaining aircraft
                            For Each acft In sortedAircraft
                                      r_hrs_per_wk = WorksheetFunction.Match(acft.tailNumber, Range("aircraft"), 0)
                                      c_current_wk = WorksheetFunction.Match(currentWeekId, Range("week_id"), 0)
                                      'Set hours this week to divide the remaining hours evenly between the remaining aircraft
                                      hrs_this_wk = Round(weekly_hrs / sortedAircraft.Count, 1)

                                      Range("mx_plan").Cells(r_hrs_per_wk, c_current_wk + weekly_hours_col_offset) = hrs_this_wk
                                                                                  'Set the 'Remaining' cell to the hours remaining to the next cycle DUE/DNE, depending on the calculateToDNE flag
                                      If calculateToDNE Then
                                              Range("mx_plan").Cells(r_hrs_per_wk, c_current_wk + remaining_col_offset) = acft.hoursToDNE - hrs_this_wk
                                      Else
                                              Range("mx_plan").Cells(r_hrs_per_wk, c_current_wk + remaining_col_offset) = acft.hoursToDUE - hrs_this_wk
                                      End If

                                      'Update acft instance for completion of week
                                      unSortedAircraft.Item(acft.tailNumStr).weeklyUpdate (hrs_this_wk)
                            Next acft

                    Case Else 'Two or more in heavy! throw/handle error
                            Debug.Print "Warning: # in heavy greater than 1: " & numberInHeavy(sortedAircraft)

          End Select

  End Sub

  'Returns the deploymemt type or fixed flight hour type as a string, or an empty string
  Private Function deploymentType(acft As c_Aircraft, weekId As Integer) As String
          deploymentType = Range("mx_plan").Cells(WorksheetFunction.Match(acft.tailNumber, Range("aircraft")), WorksheetFunction.Match(weekId, Range("week_id")))
  End Function

  'Returns an integer representing the number of aircraft in heavy maintenance
  Private Function numberInHeavy(aircraftCollection As Collection) As Integer
          Dim acft As c_Aircraft
          numberInHeavy = 0
          For Each acft In aircraftCollection
                    If acft.inHeavy Then numberInHeavy = numberInHeavy + 1
          Next acft
  End Function

  'Clear the second and third columns of Range("mx_cycles")
  Private Sub clearMxCycles(startTimer As Boolean)
          Dim i As Integer, j As Integer
          Dim StartTime As Double
          Dim SecondsElapsed As Double

          If startTimer Then
                    'Remember time when macro starts
                    StartTime = timer
          End If

          Application.ScreenUpdating = False
          For i = 2 To Range("mx_plan").Columns.Count Step 3
                    'For j = 1 To Range("mx_plan").Rows.Count
                            Range("mx_plan").Columns(i).ClearContents
                    'Next j
          Next i

          For i = 3 To Range("mx_plan").Columns.Count Step 3
                    'For j = 1 To Range("mx_plan").Rows.Count
                            Range("mx_plan").Columns(i).ClearContents
                    'Next j
          Next i
          Application.ScreenUpdating = True

          If startTimer Then
                    'Determine how many seconds code took to run
                    SecondsElapsed = Round(timer - StartTime, 4)
                    Debug.Print "This code ran successfully in " & SecondsElapsed & " seconds"
          End If

  End Sub

  Function cellIsInVisibleRange(cell As Range)
                    cellIsInVisibleRange = Intersect(ActiveWindow.VisibleRange, cell) Is Nothing
  End Function

TODO:

  1. Tackle bug in hours not correctly updating the hours of c_Aircraft instance in the Aircraft Collection following a pass through calculateWeek
  2. Tackle bug in handling when an aircraft completes it's heavy cycle (not getting out of heavy)
  3. Generate a warning when unable to fly the planned hours per week (Maybe conditional formatting?)
  4. Implement general error handling to catch unanticipated errors
  5. Create user friendly interface
\$\endgroup\$

2 Answers 2

6
\$\begingroup\$

I'm going to go for the low hanging fruit first and then try to comb through the code.

Structure - It's good practice to indent all of your code that way Labels will stick out as obvious. I'm not sure if you did that and the code block removed it or not.

Personal preference - I like variables all defined on their own line as it makes it easier to read rather than several dim-ed together. In the same breath I'll say great job on always giving variables a type even if they are declared on the same line. A lot of people make the mistake of only using a type once.

Standard VBA naming conventions have camelCase for local variables and PascalCase for other variables and names. Constants are usually Const ALLCAPS.

Speaking of Variable names - you've given your variables meaningful names, but they are difficult to work out. Why use col instead of column? Personal preference - I avoid variables like i and j because they aren't descriptive. But, there's technically nothing wrong with using them as it's standard practice to iterate with i. All of the underscores _ look messy to me as I'm only familiar with VBA conventions.

There's a lot of Worksheets("Matrix Inputs").Range("maintenance_cycles").Cells(1, 3).

Why not use the CodeName for the sheets? Worksheets have a CodeName property - View Properties window (F4) and the (Name) field can be used as the worksheet name. This way you can avoid Sheets("mySheet") and instead just use mySheet. At the same time you keep using .cells() - there has to be a better way. You're already using named ranges, so why the need for .Cells()? At least on the ones that aren't iterating.

Overall with your variables, they are pretty good, but there's no consistency. Some are like c_aircraft others are like startWeek1 and even some are like CInt.

A+ on using Option Explicit. The same goes for not using any .Select or .Activate statements.

As well as always using ByVal instead of ByRef, but ByRef is the default when not passing explicitly ByVal.

Integers - integers are obsolete. According to msdn VBA silently converts all integers to long.

Comments (though with the complexity I may off) - "code tells you how, comments tell you why". The code should speak for itself, if it needs a comment, it might need to be made more clear. If not, the comment should describe why you're doing something rather than how you're doing it. Here are a few reasons to avoid comments all together.


Maybe RubberDuck has some things to say as well.


For things like this

            If cellIsInVisibleRange(Range("week_id").Cells(1, WorksheetFunction.Match(endWeek, Range("week_id"), 0))) Then
            If WorksheetFunction.Match(endWeek, Range("week_id"), 0) < viewportOffset Then
                    Application.Goto Range("week_id").Cells(1, 1), True
            Else
                    Application.Goto Range("week_id").Cells(1, WorksheetFunction.Match(endWeek, Range("week_id"), 0) - viewportOffset), True
            End If
  End If

There's just way too much going on here to keep track of - what are the Gotos doing? They go to a particular cell? I don't understand that at all, but that might be because I have no data to test.

When you are doing two loops back to back like this -

  For i = 2 To Range("mx_plan").Columns.Count Step 3
            'For j = 1 To Range("mx_plan").Rows.Count
                    Range("mx_plan").Columns(i).ClearContents
            'Next j
  Next i

  For i = 3 To Range("mx_plan").Columns.Count Step 3
            'For j = 1 To Range("mx_plan").Rows.Count
                    Range("mx_plan").Columns(i).ClearContents
            'Next j
  Next i

Why not just combine them?

  For i = 2 To Range("mx_plan").Columns.Count Step 3
            
                    Range("mx_plan").Columns(i).ClearContents
                    Range("mx_plan").Columns(i + 1).ClearContents
            
  Next i

I'm having a pretty difficult time figuring out how your table works. Where do you keep the current number of hours on the plane and cycle ID? Or are they stored and just not shown in the image? And the hours assigned per the constraints - do you assign that based on remaining hours or are those the required hours for the aircraft? If they are required, what happens if one plane is in maintenance and another needs the preventative maintenance? Does it just sit?


My approach

would be to work directly with my tailnumber table and call functions to do the work. Ignoring the exact way that you do it, it would be something like this -

Option Explicit

Sub myAircraftHours()
    Dim planeTailNumber As Long
    Dim planeTargetHours As Long
    Dim planeRemainingHours As Long
    Dim planeInMaintenance As Boolean
    Dim currentWeek As Range
    Dim plane As Range
    Dim currentWeekID As Long
    currentWeekID = 1
    Dim currentWorkWeek As Long
    
    For Each plane In Range("aircraft")
        currentWorkWeek = WorksheetFunction.Match(currentWeekID, Range("week_id"), 0)
        planeTailNumber = plane.Value
        planeTargetHours = plane.Offset(, 2 * currentWorkWeek)
            If Not IsNumeric(plane.Offset(, 3 * currentWorkWeek)) Then calculateMaintenace planeTailNumber
        planeRemainingHours = plane.Offset(, 3 * currentWorkWeek)
        
    Next plane
     

End Sub

Function calculateMaintenace(ByVal tailNumber As Long) As Long
    'work out maintenace
End Function

Since I have everything there already, I wouldn't need a class (mostly because I don't use know classes). offset isn't really the greatest way - you probably have named ranges you could use with the currentWorkWeek variable.

If the plane is in maintenance - go see how long it will stay. If it gets out - bring it back in to having targetHours. Etc.

The For Next loop would probably get tricky with planes going in and out of circulation, so maybe you create an array and store those sorted by remaining hours, then go and match the tail numbers in the order you want.

I know my example is really simple compared to what is all going on in your macro, but that's how I would approach it.

\$\endgroup\$
9
  • \$\begingroup\$ I'll run it through the current Rubberduck 2.0 build tonight... latest release 1.4.3 has a number of resolver bugs that result in false positives with some code inspections - procedure weeklyUpdate is used in a number of places that 1.4.3 doesn't see, for example. \$\endgroup\$ Commented Feb 22, 2016 at 17:48
  • \$\begingroup\$ . @Raystafarian - thanks for the review and reply. Should I answer your questions as a comment to your response, or update my original post? \$\endgroup\$ Commented Feb 23, 2016 at 3:34
  • \$\begingroup\$ The current number of hours on the plane and cycle ID are properties of each instance of c_Aircraft in the "unsorted" Collection Aircraft`. In a given week, hours are calculated for each aircraft based on the planned total hours for the fleet, the need to ensure an individual aircraft has enough hours to spread among the remaining weeks until it enters it's next cycle, and any individual aircraft's fixed hours to fly that week. \$\endgroup\$ Commented Feb 23, 2016 at 19:55
  • \$\begingroup\$ The end goal of the program is to determine both when you run out of hours and two aircraft are in a maintenance cycle in the same week, or when you can't meet the planned hours for the fleet in a given week (e.g.run out of hours) by not allowing more than one aircraft in a cycles at the same time. \$\endgroup\$ Commented Feb 23, 2016 at 19:55
  • \$\begingroup\$ As for the cellIsInVisibleRange, I was attempting to have the viewport scroll to the column (week) where the calculation ended, if it was out of view. Hack-Isa? Yes. Functional? Sort of. Definitely a place to implement improvements. \$\endgroup\$ Commented Feb 24, 2016 at 0:02
3
\$\begingroup\$

I usually manage most of the situation using built-in formulas. This keeps the user engaging. Only when there is an event driven requirement, I create a module / class to accomplish the tasks.

I am not sure why you have opted for a Class Module design.

However, the following are my recommendations, though not fully complete:

  1. Set the property Application.ScreenUpdating to False. This will avoid refreshing your screen and process your code faster
  2. For sorting, you could use the Range.Sort function to accomplish the task
  3. To improve the looping structure, follow these principles

Bad:

     For i = 1 to Range("aircraft").count
         ....
     Next i

Good:

     TotalAircrafts = Range ("Aircraft").Count
     For AircraftCnt = 1 to TotalAircrafts
      .....
     Next AircraftCnt

Apart, if you share the workbook, I could run through and see if I could move the stone further.

\$\endgroup\$
1
  • \$\begingroup\$ See edits to post and comments on @Raystafarian 's answer for clarification \$\endgroup\$ Commented Feb 23, 2016 at 19:57

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