Help cleaning up an IF-THEN-ELSE Nightmare 1

[JasonEnsor]

Hi Guys,

I have some working code that I am ashamed of, it's a complete mess but it does the job. Just wondering if anyone has any thoughts on how I can clean it up a little.

The scenario is that payment data is loaded in to a payments userform, the userform consists of 1 textbox for each sunday of the given month (so mostly 4 boxes, occasionally 5). If the payments are edited (an example would be a user paid for a weeks swimming, but was taken ill so we change from P (Paid), to PH (Paid Holiday)) A second userform popsup allowing us to carryforward a payment to the next month, normally this would carry over to the 1st week of the next month, however occasionally the swimmer might have a holiday for the 1st week of the next month. in that case we would carry forward to the next available week. As stated this code works but it's an IF - Else nightmare in my opinion.

Public Sub UserForm_Activate()

Dim Counter As Integer
Dim WeekLabel As String
Dim MonthValue As Integer

MonthValue = GetMonthNumber(frmCarriedForward.cmbMonthList.value)
For Counter = 1 To util.maxWeeks
        WeekLabel = "chkW" & CStr(Counter)
        nthDay = NthDayOfWeek(frmCarriedForward.cmbYearList.value, MonthValue, Counter, 1)
        
        With Me.Controls(WeekLabel)
            .Visible = True
            .Caption = Str(nthDay)
        End With
        
        If util.maxWeeks = 4 Then
            WeekLabel = "chkW" & CStr(Counter + 1)
            With Me.Controls(WeekLabel)
                .Visible = False
                .Caption = ""
            End With
            txtWk5.Visible = False
        End If
    Next Counter
    
    If txtWk1.Text = "" Or txtWk1.Text = "NP" Then
        txtWk1.Text = "CF"
        txtWk1.Enabled = False
        chkW1.Enabled = True
        chkW1.value = True
    Else
        chkW1.value = False
        chkW1.Enabled = False
        
        If txtWk2.Text = "" Or txtWk2.Text = "NP" Then
            txtWk2.Text = "CF"
            txtWk2.Enabled = False
            chkW2.Enabled = True
            chkW2.value = True
        Else
            chkW2.value = False
            chkW2.Enabled = False
            
            If txtWk3.Text = "" Or txtWk3.Text = "NP" Then
            
                txtWk3.Text = "CF"
                txtWk3.Enabled = False
                chkW3.Enabled = True
                chkW3.value = True
            Else
                chkW3.value = False
                chkW3.Enabled = False
                
                If txtWk4.Text = "" Or txtWk4.Text = "NP" Then
                   txtWk4.Text = "CF"
                   txtWk4.Enabled = False
                   chkW4.Enabled = True
                   chkW4.value = True
                Else
                   If util.maxWeeks > 4 Then
                   
                      chkW5.value = False
                      chkW5.Enabled = False
                      
                      If txtWk5.Text = "" Or txtWk5.Text = "NP" Then
                         txtWk5.Text = "CF"
                         txtWk5.Enabled = False
                         chkW5.Enabled = True
                         chkW5.value = True
                      Else
                         cmbMonthList.SetFocus
                      End If
                    Else
                        cmbMonthList.SetFocus
                    End If
                End If
        
          End If
    End If
    End If
End Sub

Any thoughts on improvements would be appreciated as always.

Regards

J.

 

[mintjulep]

Perhaps something along the lines of

Public Weekly(WeekOfInterest as integer)
Dim txtControl as Control
Dim chControl as Control

Set txtControal = "txtWk" & Cstr(WeekOfInterest)
Set chControl = "chWk" & Cstr(WeekOfInterest)

For counter = 1 to 4

If counter = WeekOfInterest then
txtControl.Text = "CF"
txtControl.Enabled = False
chControl.Enabled = True
chControl.value = True
Else
chControl.value = False
chControl.Enabled = False
txtControl.Text = "CF"
txtControl.Enabled = False
chControl.Enabled = True
chControl.value = True
End if
next counter
End

That won't work of course, but conceptually....

 

[JasonEnsor]

Hi,

Thank you for the idea Mintjulep, i like your train of thought. I think after a few slight tweaks your code might just do the trick. I was half expecting someone to reply with "Just delete it and start again", i don't think i would blame them either.

Thanks again

J.