Help Tidying up some Excel VBA Code 2

[JasonEnsor]

I have been trying to think if it is possible to tidy up some of my code for a project i have been working on for a while, i have managed to do some tweaks however i am still wanting to do more.

I have a form with 4-5 comboboxes (depending on how many weeks in a month there is). each combo box holds the same information (A,C,AH) which relate to a case statement to take action for each week. for example

Private Sub cmbW1_Change()
    
    Dim txtHols
    Dim Counter
    Select Case cmbW1.Value
        Case "AH"
            For Counter = 1 To 8
                txtHols = "txtHol" & CStr(Counter)
                With Me.Controls(txtHols)
                    If .Value = "" Then
                        .Value = LblWeek1.Caption
                        Exit Sub
                    ElseIf Counter = 8 Then
                        MsgBox "No Holidays Left"
                        cmbW1.Value = "A"
                    End If
                End With
            Next Counter
        Case "PH"
            Dim nxtMonth
            Dim SwimmerPayments As Workbook
            Set SwimmerPayments = Workbooks.Open(SavePath & cmbDayList & ".xlsm")
            
            Dim SwimmerDetails As Worksheet

            Dim Rng As Range
            Dim iRow As Long
            Dim TempRow As Integer
            Set SwimmerDetails = ThisWorkbook.Sheets("Swimmer Details")
            
            util.MonthNumber = NewPaymentForm.cmbMonthList.Value
            nxtMonth = util.MonthNumber + 1
            util.MonthName = nxtMonth
            
            Set SwimmerPayments = Workbooks.Open(SavePath & cmbDayList & ".xlsm")
            Set Carry = SwimmerPayments.Sheets(util.MonthName)
            
            With Carry.Range("A:A")
                Set Rng = .Find(What:=SwimmerT.SwimmerName, _
                        After:=.Cells(.Cells.Count), _
                        LookIn:=xlValues, _
                        LookAt:=xlWhole, _
                        SearchOrder:=xlByRows, _
                        SearchDirection:=xlNext, _
                        MatchCase:=True)
            End With
    
            If Not Rng Is Nothing Then
                Application.Goto Rng, True
                TempRow = ActiveCell.Row
                
                Carry.Range("D" & TempRow) = "CF"
                Carry.Range("I" & TempRow) = NewPaymentForm.LblWeek1.Caption
        
        ' ------------------------------------------ Look at this ----------------------------------------------- '
                Carry.Range("J" & TempRow) = util.NthDOW(cmbYearList, util.MonthNumber + 1, 1, 1)
        
            Else
                
                iRow = Carry.Cells(Rows.Count, 1).End(xlUp).Offset(1, 0).Row
        
                Carry.Cells(iRow, 1).Value = NewPaymentForm.NameBox.Value
                Carry.Cells(iRow, 2).Value = NewPaymentForm.cmbClassTimes.Value
                ' Amount Paid
                '.Cells(iRow, 3).Value = NewPaymentForm.NameBox.Value
                Carry.Cells(iRow, 4).Value = "CF"
                Carry.Cells(iRow, 9).Value = NewPaymentForm.LblWeek1.Caption
                
        ' ------------------------------------------ Look at this ----------------------------------------------- '
                Carry.Cells(iRow, 10).Value = util.NthDOW(cmbYearList, util.MonthNumber + 1, 1, 1)
            End If
        
' Close Workbook
            SwimmerPayments.Save
            SwimmerPayments.Close
            SwimmerDetails.Activate
            
            txtCredits.Value = VBA.Format(SwimmerFee + txtCredits.Value, "#,##0.00")
    
    Case "P"
        MsgBox "Add: £" & VBA.Format(SwimmerFee, "#,##0.00") & " to Total"
    End Select

I just feel that i am repeating a lot of code as i would be using that 5 times in my code just so i can have a change method for each combobox. Any thoughts? As i say i am trying to go over my code and clean it up as i am finding a few inconsistancies where i have changed values for one change event and not another.

Thanks in advance

J.

 

[combo]

One of possible solutions is to create a procedure with an arguments defining the week and combobox selection. Each of event procedures can now call the same procedure - one line of code inside.

combo

 

[JasonEnsor]

That is such a simple answer and i am shocked that it wasn't my first thought. I will give that a try

Cheers Combo

 

[JasonEnsor]

Well my code has greatly reduced in size now to

 Private Sub cmbW1_Change()
    Select Case cmbW1.Value
        Case "AH"
            Call AH(1)
        Case "PH"
            Call PH(1)
    Case "P"
        Call P(1)
    End Select
End Sub

Thanks again Combo

 

[Andrzejek]

Also, how about:
[tt]
Dim Counter [blue]As Integer[/blue]
Dim txtHols [blue]As String[/blue]

Dim nxtMonth [blue]As ???[/blue]
[/tt]
etc.
Instead of all of them [tt]As Variant[/tt]

BTW - you should have installed MSTools, it will pick those declared but not used variables for you, like [tt]nxtMonth[/tt]. Pretty nice feature along many, many others.

Have fun.

---- Andy

 

[JasonEnsor]

Cheers Andy, I will check out that link I think using Variant was me being lazy....I am changing them now though