General Code Question

[LinusIT]

My programming skills are somewhat limited and I'm wondering if there is a better way of writing the following? I'm just interested to learn and make better code

Private Sub Preview_Click()
On Error GoTo Err_Preview_Click
        If IsNull([AccountCustomer]) Or [AccountCustomer] = "" Then
        MsgBox "Please select a Customer", vbInformation + vbOKOnly, "Error"
        DoCmd.GoToControl "AccountCustomer"
    Else
        If IsNull([Month]) Or [Month] = "" Then
        MsgBox "Please select a Month", vbInformation + vbOKOnly, "Error"
        DoCmd.GoToControl "Month"
    Else
        If IsNull([Year]) Or [Year] = "" Then
        MsgBox "Please select a Year", vbInformation + vbOKOnly, "Error"
        DoCmd.GoToControl "Year"
    Else
        DoCmd.OpenReport "r_account_statements", acPreview, , "CustID LIKE '" & Me.AccountCustomer & "'"
    End If
    End If
    End If
Exit_Preview_Click:
    Exit Sub
Err_Preview_Click:
    If Err = 2501 Then
    Else
        MsgBox "Error# " & Err.Number & " " & Err.Description
    End If
    Resume Exit_Preview_Click
   
End Sub
Private Sub Print_Click()
On Error GoTo Err_Print_Click
        If IsNull([AccountCustomer]) Or [AccountCustomer] = "" Then
        MsgBox "Please select a Customer", vbInformation + vbOKOnly, "Error"
        DoCmd.GoToControl "AccountCustomer"
    Else
        If IsNull([Month]) Or [Month] = "" Then
        MsgBox "Please select a Month", vbInformation + vbOKOnly, "Error"
        DoCmd.GoToControl "Month"
    Else
        If IsNull([Year]) Or [Year] = "" Then
        MsgBox "Please select a Year", vbInformation + vbOKOnly, "Error"
        DoCmd.GoToControl "Year"
    Else
        DoCmd.OpenReport "r_account_statements", acViewNormal, , "CustID LIKE '" & Me.AccountCustomer & "'"
    End If
    End If
    End If
Exit_Print_Click:
    Exit Sub
Err_Print_Click:
    If Err = 2501 Then
    Else
        MsgBox "Error# " & Err.Number & " " & Err.Description
    End If
    Resume Exit_Print_Click
End Sub

As you can see it's duplicated, I'm sure there's a better way, I just don't know what it is.

 

[pbaldy]

A slightly more efficient test for both Null and ZLS is:

If Len([AccountCustomer] & vbNullString) = 0 Then

But to the point at hand, I'd go one of two ways. If you want to keep two buttons, put the code into a function that accepts a parameter. Then simply call that function from each button, each passing a different parameter (to print or preview). Another method is to have a single button along with a checkbox. The user checks the box to print, leaves unchecked to preview (or the reverse if you prefer). Within the button code you print/preview based on the checkbox. Hint: the options for printing/previewing have numeric equivalents you can use with either method.

 

[ChrisO]

An example of the way Paul has spoken of and a few other things.
You have some reserved words in there so best get rid of them as well.
Also the error and exits labels can be the same in all procedures.

But it can boil down to style…

Option Explicit
Option Compare Text


Private Sub Preview_Click()

    On Error GoTo ErrorHandler
        
    Validate acPreview
    
ExitProcedure:
    Exit Sub
    
ErrorHandler:
    MsgBox "Error# " & Err.Number & " " & Err.Description
    
    Resume ExitProcedure
   
End Sub



Private Sub Print_Click()

    On Error GoTo ErrorHandler
        
    Validate acViewNormal
    
ExitProcedure:
    Exit Sub
    
ErrorHandler:
    MsgBox "Error# " & Err.Number & " " & Err.Description
    
    Resume ExitProcedure
   
End Sub



Private Sub Validate(ByVal lngReportAction As Long)

    On Error GoTo ErrorHandler

    Select Case True
        Case Len(Me.txtAccountCustomer & "") = 0
            MsgBox "Please select a Customer", vbInformation + vbOKOnly, "Error"
            Me.txtAccountCustomer.SetFocus
        
        Case Len(Me.txtMonth & "") = 0
            MsgBox "Please select a Month", vbInformation + vbOKOnly, "Error"
            Me.txtMonth.SetFocus
            
        Case Len(Me.txtYear & "") = 0
            MsgBox "Please select a Year", vbInformation + vbOKOnly, "Error"
            Me.txtYear.SetFocus
    
        Case Else
            DoCmd.OpenReport "r_account_statements", lngReportAction, , "CustID LIKE '" & Me.txtAccountCustomer & "'"
    End Select

ExitProcedure:
    Exit Sub
    
ErrorHandler:
    If Err <> 2501 Then
        MsgBox "Error# " & Err.Number & " " & Err.Description
    End If
    
    Resume ExitProcedure

End Sub

 

[LinusIT]

That's just what I was looking for ChrisO, thanks for taking the time in writing that for me. I'll go through my other forms and tidy that Print/Preview VBA up as well now Cheers