Help cleaning up this code (1 Viewer)

browninaz

Ugly Data Hoarder
Local time
Today, 10:30
Joined
Oct 26, 2012
Messages
88
Hello Everyone,

Just wanted to know if there is a better way to write this procedure:



Private Sub cmdCancel_Click()

'This line does not allow the user to close form if any field is populated

If Not IsNull(Me.ClientFirst) Or Not IsNull(Me.ClientLast) Or Not IsNull(Me.ClientMainPhone) _
Or Not IsNull(Me.ClientOtherPhone) Or Not IsNull(Me.ClientEmailAddress) Then

MsgBox "Please Delete and Close", , "Unable to Cancel"
Cancel = True


'This line allows the user to close the form if no field has been populated

ElseIf IsNull(Me.ClientFirst) Or IsNull(Me.ClientLast) Or IsNull(Me.ClientMainPhone) _
Or IsNull(Me.ClientOtherPhone) Or IsNull(Me.ClientMainPhone) Or IsNull(Me.ClientEmailAddress) Then

DoCmd.Close

End If
End Sub
 

plog

Banishment Pending
Local time
Today, 12:30
Joined
May 11, 2011
Messages
11,611
1. Isn't there only 2 possible outcomes of your code? Either the If portion triggers or the ElseIf trigger portion triggers, right? If so, that means no need for an ElseIf and another test. Just use an Else--you've already tested, no need to test again.

2. You can use just one Not IsNull call by combining all your inputs together:

Not IsNull(Input1 & Input2 & Input3...)
 

browninaz

Ugly Data Hoarder
Local time
Today, 10:30
Joined
Oct 26, 2012
Messages
88
Thank you, that is exactly what I was looking for.

Love this forum!
 

isladogs

MVP / VIP
Local time
Today, 17:30
Joined
Jan 14, 2017
Messages
18,186
This seems very odd practice.
If any data is entered you can't close the form whereas if its empty you can
Why would this ever be used?

Anyway there are several issues with your code:
1. You need to trap for empty strings as these won't be null
2. You need to use 'And' instead of 'Or' so that you know ALL boxes are empty to close the form
3. You only need one if else statement

Try this:

Code:
If Nz(Me.ClientFirst,"")="" And Nz(Me.ClientLast,"") ="" And Nz(Me.ClientMainPhone,""="") _
And Nz(Me.ClientOtherPhone,"")="" And Nz(Me.ClientMainPhone,"") ="" And Nz(Me.ClientEmailAddress,"")="" Then
	DoCmd.Close
Else
	MsgBox "Please Delete and Close", , "Unable to Cancel"
End If

I'm sure if could be simplified further

EDIT: just saw Plog's reply & it has been!
 

Users who are viewing this thread

Top Bottom