Transferring data for a given month from one sheet to another I have dollar amounts sorted by month that I want to transfer from sheet entry to sheet actuals. I only want to do it for a specified month prompted from user. Each dollar amount has a corresponding cost center/ID and description in separate columns. I want to copy the amounts and description from entry to the row in actuals matching the same ID.

Sometimes, multiple amounts share the same ID so I need to place them one below the other.

Sub month()

Dim actualsWS As Worksheet
Dim dataWS As Worksheet
Dim answer As String
Dim month As String
Dim loc As Range
Dim start As Integer
Dim rowCol As Variant
Dim dropRow As Integer
Dim dropCol As Integer
Dim locActual As Range

'store worksheets into variables
Set actualsWS = ThisWorkbook.Worksheets("Actuals by Month")
Set dataWS = ThisWorkbook.Worksheets("FAS Data Entry")

MsgBox ("If macro dies, it's likely a missing cost center.") 'warning for user

answer = InputBox(prompt:="What month?") 'prompt user for current month

'set month from user input
Select Case answer

    Case 1
    month = "January"
    'dropCol = 9
    Case 2
    month = "February"
    'dropCol = 10
    Case 3
    month = "March"
    'dropCol = 11
    Case 4
    month = "April"
    'dropCol = 12
    Case 5
    month = "May"
    'dropCol = 13
    Case 6
    month = "June"
    'dropCol = 14
    Case 7
    month = "July"
    'dropCol = 15
    Case 8
    month = "August"
    'dropCol = 16
    Case 9
    month = "September"
    'dropCol = 17
    Case 10
    month = "October"
    'dropCol = 18
    Case 11
    month = "November"
    'dropCol = 19
    Case 12
    month = "December"
    'dropCol = 20
    Case Else
    MsgBox ("Bad month. Exiting...")
    Exit Sub

End Select

Dim ccArray(99) 'cost center
Dim amountArray(99) As Currency 'amount
Dim descriptArray(99) 'description
Dim ccArrayLength As Integer

With dataWS

    'locate starting range for current month
    Set loc = .Cells.Find(month)

    'check previous row for carried over month (revising asset) Will break if 2 or more
    If loc.Offset(-1, 0) = "" Then 'if previous row is blank
        start = loc.row 'starting row is same row as found location above
    Else
        start = loc.row - 1 'else it's 1 row above
    End If

    i = 0

    'store data into arrays
    Do While .Cells(start + i, 1) &lt;&gt; "" 'loop through column A while there is a month present (not blank)

        ccArray(i) = .Cells(start + i, 11).Value 'store cost center
        amountArray(i) = .Cells(start + i, 13).Value 'store amount
        descriptArray(i) = .Cells(start + i, 9).Value 'store description

        i = i + 1

    Loop

End With

ccArrayLength = Application.Count(ccArray) 'store number of items

With actualsWS

    .Columns("V:W").ClearContents 'clear previous data

    'hide unnecessary months
    If month = "January" Or month = "February" Or month = "March" Or month = "April" Then
        .Columns("M:Q").EntireColumn.Hidden = True
    End If

    'drop data into respective cells onto "actuals by month" tab
    For x = 0 To ccArrayLength - 1

        On Error Resume Next
        Set locActual = .Cells.Find(ccArray(x)) 'find cost center location
        If locActual = 0 Then GoTo died 'not found
        'Debug.Print locActual
        'rowCol = locActual.Address
        dropRow = .Range(locActual.Address).row 'store row of cost center

        'for loop to find first empty cell to drop into
        For y = 0 To 99 'no cost center should have over 99 rows

            If .Cells(dropRow + y, 22).Value = "" Then 'column v blank
                .Cells(dropRow + y, 22).Value = amountArray(x) 'drop amount
                .Cells(dropRow + y, 22).Offset(0, 1).Value = descriptArray(x) 'drop description on row below
                Exit For
            End If

        Next

    Next

    .Cells(8, 22).Select 'bring cursor to top

End With

died:
'quit before msgbox if on last item
If x = ccArrayLength Then Exit Sub

MsgBox ccArray(x) &amp; " not found." 'cost center not found
Exit Sub

End Sub
Sub month()

Dim month As String



Something isn't right, one of these two has the wrong name. Procedure names should start with a verb, they do something. If you can name that something, then you can name the procedure. If you can't name that something, then your procedure is doing too many things and needs to be broken down into something that can be named.

So we're getting the month name for some user input - if that's always going to be in English, then there's no need to hard-code that in a Select Case block.

Public Function ToMonthName(ByVal value As Integer) As String
    On Error GoTo CleanFail 'remove if runtime error 5 should bubble up to caller instead

    Dim result As String
    result = MonthName(value)

CleanExit:
    ToMonthName = result
    Exit Function
CleanFail:
    result = vbNullString
    Resume CleanExit
End Function


So, now we have the month name (or an empty string), that month procedure can actually start focusing on its task.

It's confusing that every code paths execute the died: label. There's no error handling - seeing that line label I was expecting to see On Error GoTo died somewhere.

So two things are happening here. First we're iterating dataWS and populating three arrays in parallel - there's a design smell right here: you have 3 data structures that are "in sync", when there should really be only one.

Make a small class module, and expose the 3 properties you're interested in:

Option Explicit
Private Type TModel
    CostCenter As String
    Description As String
    Amount As Single
End Type

Private this As TModel

Public Property Get CostCenter() As String
    CostCenter = this.CostCenter
End Property

Public Property Let CostCenter(ByVal value As String)
    this.CostCenter = value
End Property

Public Property Get Description() As String
    Description = this.Description
End Property

Public Property Let Description(ByVal value as String)
    this.Description = value
End Property

Public Property Get Amount() As Single
    Amount = this.Amount
End Property

Public Property Let Amount(ByVal value As Single)
    this.Amount = value
End Property


Now you can store instances of this class, which encapsulates the data you're interested in. This:


Do While .Cells(start + i, 1) &lt;&gt; "" 'loop through column A while there is a month present (not blank)

    ccArray(i) = .Cells(start + i, 11).Value 'store cost center
    amountArray(i) = .Cells(start + i, 13).Value 'store amount
    descriptArray(i) = .Cells(start + i, 9).Value 'store description

    i = i + 1

Loop



Becomes this:

Dim data As Collection
Set data = New Collection

Dim item As DataModel 'assuming class name was "DataModel"

Do While .Cells(start + i, 1) &lt;&gt; vbNullString 'loop through column A while there is a month present (not blank)

    Set item = New DataModel
    item.CostCenter = .Cells(start + i, 11).Value
    item.Amount = .Cells(start + i, 13).Value
    item.Description = .Cells(start + i, 9).Value

    data.Add item        
    i = i + 1

Loop


And now if the body of that loop was extracted into its own function...

Private Function ReadModel(ByVal source As Range) As DataModel
    Dim result As New DataModel
    result.CostCenter = source.Cells(1, 11).Value
    result.Amount = source.Cells(1, 13).Value
    result.Description = source.Cells(1, 9).Value
    Set ReadModel = result
End Function


You could also define an Enum to get rid of the magic values there:

Private Enum ColumnPosition
    DescriptionColumn = 9
    CostCenterColumn = 11
    AmountColumn = 13
End Enum


And then use them to enhance readability here:

Private Function ReadModel(ByVal source As Range) As DataModel
    Dim result As New DataModel
    result.CostCenter = source.Cells(1, CostCenterColumn).Value
    result.Amount = source.Cells(1, AmountColumn).Value
    result.Description = source.Cells(1, DescriptionColumn).Value
    Set ReadModel = result
End Function


The calling code will now look like this:

Dim data As Collection
Set data = New Collection

Do While .Cells(start + i, 1) &lt;&gt; vbNullString 'loop through column A while there is a month present (not blank)

    data.Add ReadModel(.Range("A" &amp; start + i).EntireRow)
    i = i + 1

Loop


And now if you ever need to read more values, that code doesn't even need to change - you can simply modify the DataModel accordingly, and the function that reads a row's values into an instance. And this loop and collection should go into its own function, too.

The number of items in the collection would be simply data.Count.

Next the loop that consumes this data, could be a For Each loop:

Dim item As DataModel
For Each item In data
    '...
Next


Again, this should be in its own procedure. On Error Resume Next shouldn't need to be there. What errors are being thrown under the carpet? They should be handled.