Use of Goto Exit_Proc

From http://www.mvps.org/access/tencommandments.htm

9th item:

Thou shalt not use "SendKeys", "Smart Codes" or "GoTo" (unless the GoTo
be part of an OnError process) for these will lead you from the path of
righteousness.

What about also using it as a means of exiting a procedure?

I'm a firm believer that exit sub and exit function should not be
sprinkled about tany procedure. There should one, and only one exit point.

For example:

Function fSilly() as Boolean

'returns true if the remarks are silly

dim int1 as integer 'just to flesh out the proc

On Error Goto Err_Proc

int1 = forms!frmSIlly.txtSIllyIndicator

if int1 = 0 then

'perform all kinds of stuff

elseif int1 = 1 then

GoTo Exit_Proc

end if

Exit_Proc:

'set database variables to nothing, etc

Exit_Sub

Err_Proc:

Select case err.number

case else

'display error message

goto exit_proc

end select

--
Tim http://www.ucs.mun.ca/~tmarshal/
^o<
/#) "Burp-beep, burp-beep, burp-beep?" - Quaker Jake
/^^ "What's UP, Dittoooooo?" - Ditto

re: Use of Goto Exit_Proc

"Tim Marshall" <TIMMY!@PurplePandaChasers.Moertherium> wrote in message
news:cv5j4t$ede$1@coranto.ucs.mun.ca...[color=blue]
> From http://www.mvps.org/access/tencommandments.htm
>
> 9th item:
>
> Thou shalt not use "SendKeys", "Smart Codes" or "GoTo" (unless the GoTo be
> part of an OnError process) for these will lead you from the path of
> righteousness.
>
> What about also using it as a means of exiting a procedure?
>
> I'm a firm believer that exit sub and exit function should not be
> sprinkled about tany procedure. There should one, and only one exit
> point.
>
> For example:
>
> Function fSilly() as Boolean
>
> 'returns true if the remarks are silly
>
> dim int1 as integer 'just to flesh out the proc
>
> On Error Goto Err_Proc
>
> int1 = forms!frmSIlly.txtSIllyIndicator
>
> if int1 = 0 then
>
> 'perform all kinds of stuff
>
> elseif int1 = 1 then
>
> GoTo Exit_Proc
>
> end if
>
> Exit_Proc:
>
> 'set database variables to nothing, etc
>
> Exit_Sub
>
> Err_Proc:
>
> Select case err.number
>
> case else
>
> 'display error message
>
> goto exit_proc
>
> end select
>
> --
> Tim http://www.ucs.mun.ca/~tmarshal/
> ^o<
> /#) "Burp-beep, burp-beep, burp-beep?" - Quaker Jake
> /^^ "What's UP, Dittoooooo?" - Ditto[/color]




Probably better to have your own rules of coding which you stick to than to
have none at all. In the example you have given, you have hinted that there
may be objects to be closed / set to nothing. But I can imagine writing
code for that function where I wrote:

If Not IsFormLoaded("frmSilly") Then
Exit Function
End If

where I do the most basic check and exit quickly if this criterion is not
met. I know that nothing else needs doing, I know the function will return
false and so I'm happy to write it. You might insist on Goto Exit_Proc, but
it makes no difference to how the code will run in this case, but here I
would find either case acceptable.

I've seen people who might start the function with blnReturn=False, which
they claim helps readability in that they make it clear that unless
something changes, the return value will be false. Of course it would be
false with or without that line, but some (although not me) find it helpful.

While we're on the subject of coding conventions, I tend to write code as
shown below, although I have not seen many others do it:

Exit_Proc:
On Error Resume Next
If Not rst Is Nothing Then
rst.Close
Set rst=Nothing
End If
Exit Function

That is, with the exit part, the code should do it's best to clear up and
exit - hence the resume next - otherwise an error would send us round in an
eternal loop. So I do bother to check whether or not the recordset is
nothing or not, but I neglect to check whether it can be closed or not
(perhaps it was never opened) - I just steam-roller through to the exit In
other words, close it if you can, otherwise move on.

I wonder what others think of this.

re: Use of Goto Exit_Proc

On Fri, 18 Feb 2005 23:10:41 +0000 (UTC), "Stefan Kowalski" <a@b.com>
wrote:

I use an even bigger steamroller than you do. In my mind there is no
need to be subtle about one issue (is an object) while not being
subtle about another (object is open):

Exit_Proc:
On Error Resume Next
rst.Close
Set rst=Nothing
Exit Function

-Tom.

<clip>[color=blue]
>
>While we're on the subject of coding conventions, I tend to write code as
>shown below, although I have not seen many others do it:
>
>Exit_Proc:
> On Error Resume Next
> If Not rst Is Nothing Then
> rst.Close
> Set rst=Nothing
> End If
> Exit Function
>
>That is, with the exit part, the code should do it's best to clear up and
>exit - hence the resume next - otherwise an error would send us round in an
>eternal loop. So I do bother to check whether or not the recordset is
>nothing or not, but I neglect to check whether it can be closed or not
>(perhaps it was never opened) - I just steam-roller through to the exit In
>other words, close it if you can, otherwise move on.
>
>I wonder what others think of this.
>[/color]

re: Use of Goto Exit_Proc

The use of Goto has lead to such awful spaghetti in the past that common
wisdom has been to avoid using it - period. In a language with sufficiently
powerful code flow control mechanisms, this logic is probably valid.

In VB/VBA, though, there are times when the code is easier to read/follow with
a judicious use of a Goto, and in those cases, that's exactly what you should
use.

Here's the mental process I follow:

1. Get to a place where I'm tempted to use a Goto.
2. Red flag! Why do I think I need a goto?!
3. Is there some better code flow control construct for this?
3a. Exit Do? Exit For?
3b. Store a boolean state, and evaluate it later?
3c. Extract code to new procedure, and call from If block?
3d. Extract code to new procedure, and use Exit Sub/Function?
4. Nope - none of those is better than what I have now.
5. OK - use the Goto, and be proud of it.

Getting to 5 is not particularly rare.


On Fri, 18 Feb 2005 16:58:21 -0330, Tim Marshall
<TIMMY!@PurplePandaChasers.Moertherium> wrote:
[color=blue]
> From http://www.mvps.org/access/tencommandments.htm
>
>9th item:
>
>Thou shalt not use "SendKeys", "Smart Codes" or "GoTo" (unless the GoTo
>be part of an OnError process) for these will lead you from the path of
>righteousness.
>
>What about also using it as a means of exiting a procedure?
>
>I'm a firm believer that exit sub and exit function should not be
>sprinkled about tany procedure. There should one, and only one exit point.
>
>For example:
>
>Function fSilly() as Boolean
>
>'returns true if the remarks are silly
>
> dim int1 as integer 'just to flesh out the proc
>
> On Error Goto Err_Proc
>
> int1 = forms!frmSIlly.txtSIllyIndicator
>
> if int1 = 0 then
>
> 'perform all kinds of stuff
>
> elseif int1 = 1 then
>
> GoTo Exit_Proc
>
> end if
>
>Exit_Proc:
>
> 'set database variables to nothing, etc
>
> Exit_Sub
>
>Err_Proc:
>
> Select case err.number
>
> case else
>
> 'display error message
>
> goto exit_proc
>
> end select[/color]

re: Use of Goto Exit_Proc

Tim,
I've been coding for 13 years in VB and VBA and can think of only one
occasion where I have used GoTo except as part of an error handling routine.

The example you show does not require GoTo at all.

My personal advice is; if you feel the need to use goto:
1) Try calmimg down, you are obviously overworked and possibly a bit
emotional.
2) Have a cold shower
3) Consider the ridicule of your peers when they see what you have done.


<G>


--
Terry Kreft
MVP Microsoft Access


"Tim Marshall" <TIMMY!@PurplePandaChasers.Moertherium> wrote in message
news:cv5j4t$ede$1@coranto.ucs.mun.ca...[color=blue]
> From http://www.mvps.org/access/tencommandments.htm
>
> 9th item:
>
> Thou shalt not use "SendKeys", "Smart Codes" or "GoTo" (unless the GoTo
> be part of an OnError process) for these will lead you from the path of
> righteousness.
>
> What about also using it as a means of exiting a procedure?
>
> I'm a firm believer that exit sub and exit function should not be
> sprinkled about tany procedure. There should one, and only one exit[/color]
point.[color=blue]
>
> For example:
>
> Function fSilly() as Boolean
>
> 'returns true if the remarks are silly
>
> dim int1 as integer 'just to flesh out the proc
>
> On Error Goto Err_Proc
>
> int1 = forms!frmSIlly.txtSIllyIndicator
>
> if int1 = 0 then
>
> 'perform all kinds of stuff
>
> elseif int1 = 1 then
>
> GoTo Exit_Proc
>
> end if
>
> Exit_Proc:
>
> 'set database variables to nothing, etc
>
> Exit_Sub
>
> Err_Proc:
>
> Select case err.number
>
> case else
>
> 'display error message
>
> goto exit_proc
>
> end select
>
> --
> Tim http://www.ucs.mun.ca/~tmarshal/
> ^o<
> /#) "Burp-beep, burp-beep, burp-beep?" - Quaker Jake
> /^^ "What's UP, Dittoooooo?" - Ditto[/color]

re: Use of Goto Exit_Proc

Sorry for the change of topic, but I think I'm having a catharsis of
sorts here...

Terry Kreft wrote:
[color=blue]
> Tim,
> I've been coding for 13 years in VB and VBA and can think of only one
> occasion where I have used GoTo except as part of an error handling routine.
>
> The example you show does not require GoTo at all.[/color]
[color=blue]
> My personal advice is; if you feel the need to use goto:
> 1) Try calmimg down, you are obviously overworked and possibly a bit
> emotional.
> 2) Have a cold shower
> 3) Consider the ridicule of your peers when they see what you have done.
>
>
> <G>[/color]


8) 8) <sob> I've left myself wide open now...

Seriously though, how would you do it?

I used to have separate exit function/sub along with whatever closing of
variables. I began using GOTo as a means to end the procedure
prematurely if certain conditions are not meant.

About 1993, a programmer at an organization I was involved with told me
GOTO was bad in any language. I've made a large and successful effort
to avoid it by using if statements, loops and so on. However, in the
past year or so, I have started using using it only as I describe above.

Of course for ensuring conditions are met, you can use an if statement:

If <conditions met> then

do stuff

end if

Exit_Proc:

However, you would have to be very creative in the case of the example
below.

Here's a "real life" proc. It's the OK button for a form that creates a
new record. The table for the insert action has plenty and plenty of
database level constraints, plus there are some other constarints I felt
could best be handled at the form level.

I'm curious how folks would handle the verification section in which the
app checks to make sure the user has supplied specific values. It's a
large procedure, but the majority of it is to check the correct values
are there to avoid a runtime error.

I've learned an incredible amount from you and all the other folks here
over the past 7 or so years, so I'll gladly take large amounts of
ridicule 8) for the following, but am sure I'll learn a better way to do
it amidst it all!

There are a couple of statements that call on other routines to check
stuff. See If fSelectTerrain = False Then GoTo Exit_Proc.... Perhaps I
could have:

If fselectTerrain = true and <insert various other subroutine checks> then

end if

Anyway, thanks in advance! 8)

Private Sub btnOk_Click()

'This will run an insert/update for the main TBL_ACTION and a series of
INSERTS for
'TBL_ACTION_TERRAIN if this is a new record. If an old record, a delete of
'existing TBL_ACTION_TERRAIN will occur first

Dim strSql As String
Dim intC As Integer
Dim lngPk As Long
Dim rst As DAO.Recordset

On Error GoTo Err_Proc

'ensure abbreviation and name is chosen

If Nz(Me.txtAbrev, "") = "" Then

MsgBox "Enter a unique (for this campaign) abbreviation/code
for this action!"

Me.txtAbrev.SetFocus

GoTo Exit_Proc

End If

'ensure a name is selected for this action

If Trim(Nz(Me.txtName, "")) = "" Then

MsgBox "Enter a name for this action!"

Me.txtName.SetFocus

GoTo Exit_Proc

End If

'make sure an action type is selected

If IsNull(Me.fraType) Or Me.fraType = 0 Then

MsgBox "You haven't chosen an action type."

Me.fraType.SetFocus

GoTo Exit_Proc

End If

'next verify that if 11 or 12 that one or all destination list box
item is selected

If fCheckDest = False Then GoTo Exit_Proc 'a separate procedure

If fSelectTerrain = False Then GoTo Exit_Proc

'next make sure that only one value is selected

If Me.fraType = 11 Or Me.fraType = 12 Then

lngPk = 0

For intC = 0 To Me.lstDestination.ListCount - 1

If Me.lstDestination.Selected(intC) Then

lngPk = Me.lstDestination.Column(0, intC)

Exit For

End If

Next intC

If lngPk = 0 Then 'nothing chosen

MsgBox "Choose a terrain type ""effect"" for the
crossing/obstacle!"

Me.lstDestination.SetFocus

GoTo Exit_Proc

End If

End If

'is there a

'verify that there is a value in txtValue. if it is enabled, there
needs to be a value

If Me.txtValue.Enabled = True Then

If IsNull(Me.txtValue) Then

MsgBox "Enter a value!"

Me.txtValue.SetFocus

GoTo Exit_Proc

End If

End If

'missions

If Me.txtMissions.Enabled = True Then

If IsNull(Me.txtMissions) Then

MsgBox "Enter number of missions!"

Me.txtMissions.SetFocus

GoTo Exit_Proc

End If

End If

'do the action in tbl_action first and get the pk

If Forms!frmsetup.subExplain.Form.txtAddEdit = "add" Then strSql =
fInsert Else strSql = fUpdate

'Find lngPK, ACT_PK of new/updated action

If Forms!frmsetup.subExplain.Form.txtAddEdit = "edit" Then

lngPk = Forms!frmsetup.subExplain.Form.ACT_PK

Else

'just do a dmax, that will be the latest one

lngPk = DMax("ACT_PK", "TBL_ACTIONS", "ACT_GAM_FK = " &
Forms!frmsetup.txtPK)

End If

'Now do terrain restrictions

fRestrictions lngPk

'requery

Forms!frmsetup.subExplain.Form.Requery

'bookmark

Set rst = Forms!frmsetup.subExplain.Form.RecordsetClone

With rst

.MoveFirst

.FindFirst "ACT_PK = " & lngPk

Forms!frmsetup.subExplain.Form.Bookmark = .Bookmark

.MoveFirst

.Close

End With

DoCmd.Close acForm, "frmsetupActionsadd", acSaveNo

Exit_Proc:

Set rst = Nothing

Exit Sub

Err_Proc:

Select Case Err.Number

Case 3022 'index - in this case, a repaeated name

MsgBox "You already have an action with the abbreviation
""" & Me.txtAbrev & """", vbExclamation

Me.txtName.SetFocus

GoTo Exit_Proc

Case Else

MsgBox "Error " & Err.Number & " " & Err.Description,
vbCritical, "frmSetUpActionsAdd btnOk_Click", Err.HelpFile, Err.HelpContext

GoTo Exit_Proc

End Select


End Sub



--
Tim http://www.ucs.mun.ca/~tmarshal/
^o<
/#) "Burp-beep, burp-beep, burp-beep?" - Quaker Jake
/^^ "What's UP, Dittoooooo?" - Ditto

re: Use of Goto Exit_Proc

Steve Jorgensen wrote:
[color=blue]
> The use of Goto has lead to such awful spaghetti in the past that common
> wisdom has been to avoid using it - period.[/color]

Thanks teve - could I ask you to have a look at my response to Terry
Kreft in this thread and offer an opinion? I think, though, that your
check list answered my question... 8)

--
Tim http://www.ucs.mun.ca/~tmarshal/
^o<
/#) "Burp-beep, burp-beep, burp-beep?" - Quaker Jake
/^^ "What's UP, Dittoooooo?" - Ditto

re: Use of Goto Exit_Proc

On Mon, 21 Feb 2005 12:43:18 -0330, Tim Marshall
<TIMMY!@PurplePandaChasers.Moertherium> wrote:
[color=blue]
>Steve Jorgensen wrote:
>[color=green]
>> The use of Goto has lead to such awful spaghetti in the past that common
>> wisdom has been to avoid using it - period.[/color]
>
>Thanks teve - could I ask you to have a look at my response to Terry
>Kreft in this thread and offer an opinion? I think, though, that your
>check list answered my question... 8)[/color]

I could, but it will probably come down to 2 smart people who simply disagree.
The only thing I might add is to say that I make my choices pragmatically,
based on what will be easiest to read and maintain. I use certain things like
the use of GoTo as red flags to tell me I might be on the wrong track, but
with the emphasis on "might".

re: Use of Goto Exit_Proc

Tim Marshall wrote:
[color=blue]
> Sorry for the change of topic, but I think I'm having a catharsis of
> sorts here...
>
> Terry Kreft wrote:[color=green]
>> 3) Consider the ridicule of your peers when they see what you have
>> done.[/color][/color]

OK, rather than ask people to suggest code changes, perhaps Terry or
someone else can tell me if the following is OK. Personally, I don't
see a problem with the goto exit_proc approach... but I must avoid
ridicule!!! 8)

booProceed and strProceed are new variables here.

Private Sub btnOk_Click()

'This will run an insert/update for the main TBL_ACTION and a series of
INSERTS for
'TBL_ACTION_TERRAIN if this is a new record. If an old record, a delete of
'existing TBL_ACTION_TERRAIN will occur first

Dim strSql As String
Dim intC As Integer
Dim lngPk As Long
Dim rst As DAO.Recordset

Dim booProceed As Boolean
Dim strProceed As String

On Error GoTo Err_Proc

booProceed = True 'if any of the following make it false, procedure
will not go ahead
strProceed = ""

'ensure abbreviation and name is chosen

If Nz(Me.txtAbrev, "") = "" Then

strProceed = "Enter a unique (for this campaign)
abbreviation/code for this action!"

Me.txtAbrev.SetFocus

booProceed = False

End If

'ensure a name is selected for this action

If Trim(Nz(Me.txtName, "")) = "" Then

strProceed = strProceed & IIf(strProceed <> "", vbCrLf &
vbCrLf, "") & "Enter a name for this action!"

Me.txtName.SetFocus

booProceed = False

End If

'make sure an action type is selected

If IsNull(Me.fraType) Or Me.fraType = 0 Then

strProceed = strProceed & IIf(strProceed <> "", vbCrLf &
vbCrLf, "") & "You haven't chosen an action type."

Me.fraType.SetFocus

booProceed = False

End If

'next verify that if 11 or 12 that one or all destination list box
item is selected

If fCheckDest = False Then booProceed = False

If fSelectTerrain = False Then booProceed = False

'next make sure that only one value is selected

If Me.fraType = 11 Or Me.fraType = 12 Then

lngPk = 0

For intC = 0 To Me.lstDestination.ListCount - 1

If Me.lstDestination.Selected(intC) Then

lngPk = Me.lstDestination.Column(0, intC)

Exit For

End If

Next intC

If lngPk = 0 Then 'nothing chosen

strProceed = strProceed & IIf(strProceed <> "", vbCrLf &
vbCrLf, "") & "Choose a terrain type ""effect"" for the crossing/obstacle!"

Me.lstDestination.SetFocus

booProceed = False

End If

End If

'is there a

'verify that there is a value in txtValue. if it is enabled, there
needs to be a value

If Me.txtValue.Enabled = True Then

If IsNull(Me.txtValue) Then

strProceed = strProceed & IIf(strProceed <> "", vbCrLf &
vbCrLf, "") & "Enter a value!"

Me.txtValue.SetFocus

booProceed = False

End If

End If

'missions

If Me.txtMissions.Enabled = True Then

If IsNull(Me.txtMissions) Then

strProceed = strProceed & IIf(strProceed <> "", vbCrLf &
vbCrLf, "") & "Enter number of missions!"

Me.txtMissions.SetFocus

booProceed = False

End If

End If

If booProceed = False Then

'display error messsage if strProceed is not empty string,
otherwise, the
'separate routines will have displayed a message

If strProceed <> "" Then MsgBox strProceed, vbExclamation,
"Incomplete Information for This Action"

Else

'do the action in tbl_action first and get the pk

If Forms!frmsetup.subExplain.Form.txtAddEdit = "add" Then
strSql = fInsert Else strSql = fUpdate

'Find lngPK, ACT_PK of new/updated action

If Forms!frmsetup.subExplain.Form.txtAddEdit = "edit" Then

lngPk = Forms!frmsetup.subExplain.Form.ACT_PK

Else

'just do a dmax, that will be the latest one

lngPk = DMax("ACT_PK", "TBL_ACTIONS", "ACT_GAM_FK = " &
Forms!frmsetup.txtPK)

End If

'Now do terrain restrictions

fRestrictions lngPk

'requery

Forms!frmsetup.subExplain.Form.Requery

'bookmark

Set rst = Forms!frmsetup.subExplain.Form.RecordsetClone

With rst

.MoveFirst

.FindFirst "ACT_PK = " & lngPk

Forms!frmsetup.subExplain.Form.Bookmark = .Bookmark

.MoveFirst

.Close

End With

DoCmd.Close acForm, "frmsetupActionsadd", acSaveNo

End If

Exit_Proc:

Set rst = Nothing

Exit Sub

Err_Proc:

Select Case Err.Number

Case 3022 'index - in this case, a repaeated name

MsgBox "You already have an action with the abbreviation
""" & Me.txtAbrev & """", vbExclamation

Me.txtName.SetFocus

GoTo Exit_Proc

Case Else

MsgBox "Error " & Err.Number & " " & Err.Description,
vbCritical, "frmSetUpActionsAdd btnOk_Click", Err.HelpFile, Err.HelpContext

GoTo Exit_Proc

End Select


End Sub


--
Tim http://www.ucs.mun.ca/~tmarshal/
^o<
/#) "Burp-beep, burp-beep, burp-beep?" - Quaker Jake
/^^ "What's UP, Dittoooooo?" - Ditto

re: Use of Goto Exit_Proc

Tim,

Your code does allow for a lot of confusion and maintenance problems
down the line. Also, it can frustrate your users as they fix one problem
and try again only to get yet another new error message. Here is
something I've used that might help.

Bri

========Begin air code======
Form_Before_Update(Cancel as Integer)
On Error goto Err_
Dim stMSG AS String

If <test1> Then
stMSG = stMSG & "Missing Name" & vbCR
End If
If <test2> Then
stMSG = stMSG & "Missing Abbreviation" & vbCR
End If
..... do same for all tests
If Len(stMSG)>0 Then 'At least one test failed
stMSG = "Please Fix the following Problems:" & vbCR & vbCR & stMSG
MsgBox stMSG
Cancel=True
Else
'Put any additional code for when the tests pass here
End If

Exit_:
'Cleanup here
Exit Sub

Err_:
'Error stuff here
Resume Exit_
End Sub

Tim Marshall wrote:[color=blue]
> Sorry for the change of topic, but I think I'm having a catharsis of
> sorts here...[/color]
<snip>[color=blue]
>
> 8) 8) <sob> I've left myself wide open now...
>
> Seriously though, how would you do it?
>
> I used to have separate exit function/sub along with whatever closing of
> variables. I began using GOTo as a means to end the procedure
> prematurely if certain conditions are not meant.
>
> About 1993, a programmer at an organization I was involved with told me
> GOTO was bad in any language. I've made a large and successful effort
> to avoid it by using if statements, loops and so on. However, in the
> past year or so, I have started using using it only as I describe above.
>
> Of course for ensuring conditions are met, you can use an if statement:
>
> If <conditions met> then
>
> do stuff
>
> end if
>
> Exit_Proc:
>
> However, you would have to be very creative in the case of the example
> below.
>
> Here's a "real life" proc. It's the OK button for a form that creates a
> new record. The table for the insert action has plenty and plenty of
> database level constraints, plus there are some other constarints I felt
> could best be handled at the form level.
>
> I'm curious how folks would handle the verification section in which the
> app checks to make sure the user has supplied specific values. It's a
> large procedure, but the majority of it is to check the correct values
> are there to avoid a runtime error.
>
> I've learned an incredible amount from you and all the other folks here
> over the past 7 or so years, so I'll gladly take large amounts of
> ridicule 8) for the following, but am sure I'll learn a better way to do
> it amidst it all!
>
> There are a couple of statements that call on other routines to check
> stuff. See If fSelectTerrain = False Then GoTo Exit_Proc.... Perhaps I
> could have:
>
> If fselectTerrain = true and <insert various other subroutine checks> then
>
> end if
>
> Anyway, thanks in advance! 8) <code snipped>[/color]

re: Use of Goto Exit_Proc

On Mon, 21 Feb 2005 12:38:27 -0330, Tim Marshall
<TIMMY!@PurplePandaChasers.Moertherium> wrote:

.....[color=blue]
>Seriously though, how would you do it?[/color]

The following is not tested, but it should give you the idea...

Private Sub btnOk_Click()

'This will run an insert/update for the main TBL_ACTION and a
'series of INSERTS for TBL_ACTION_TERRAIN if this is a new
'record. If an old record, a delete of existing
'TBL_ACTION_TERRAIN will occur first

Dim lngPk As Long
Dim strInvalidDataMsg As String
Dim ctlInvalidEntry As Access.Control

On Error GoTo Err_Proc

If Not IsDataValid(strInvalidDataMsg, ctlInvalidEntry) Then
Exit Sub
End If

lngPk = GetPrimaryKey()
fRestrictions lngPk
Forms!frmsetup.subExplain.Form.Requery
ResyncSubExplainBookmark lngPk
DoCmd.Close acForm, "frmsetupActionsadd", acSaveNo

Exit_Proc:
Exit Sub

Err_Proc:
Select Case Err.Number
Case 3022 'index - in this case, a repaeated name
MsgBox "You already have an action with the " & _
"abbreviation """ & Me.txtAbrev & """", _
vbExclamation
Me.txtName.SetFocus

Case Else
MsgBox "Error " & Err.Number & " " & Err.Description, _
vbCritical, _
"frmSetUpActionsAdd btnOk_Click", _
Err.HelpFile, Err.HelpContext

End Select

Resume Exit_Proc

End Sub

Private Function IsBlankEntry(varEntry As Variant) As Boolean
IsBlankEntry = (Len(Trim(varEntry & ""))=0)
End Function

Private Function IsDataValid( _
ByRef strInvalidDataMsg As String, _
ByRef ctlInvalidEntry As Access.Control _
) As Boolean
Dim blnResultOk As Boolean

Dim intC As Integer
Dim lngPk As Long

If IsBlankEntry(Me.txtAbrev) Then
blnResultOk = False
strInvalidDataMsg = "Enter a unique (for this campaign" & _
") abbreviation/code for this action!"
Set ctlInvalidEntry = Me.txtAbrev

ElseIf IsBlankEntry(Me.txtName) Then
blnResultOk = False
strInvalidDataMsg = "Enter a name for this action!"
Set ctlInvalidEntry = Me.txtName

ElseIf Nz(Me.fraType, 0) = 0 Then
blnResultOk = False
strInvalidDataMsg = "You haven't chosen an action type."
Set ctlInvalidEntry = Me.fraType

ElseIf fCheckDest = False Then
blnResultOk = False 'a separate procedure

ElseIf fSelectTerrain = False
blnResultOk = False

ElseIf Me.fraType = 11 Or Me.fraType = 12 Then

lngPk = 0
For intC = 0 To Me.lstDestination.ListCount - 1
If Me.lstDestination.Selected(intC) Then
lngPk = Me.lstDestination.Column(0, intC)
Exit For
End If
Next intC

If lngPk = 0 Then 'nothing chosen
blnResultOk = False
strInvalidDataMsg = "Choose a terrain type ""effect""" & _
" for the crossing/obstacle!"
Set ctlInvalidEntry = Me.lstDestination
End If

ElseIf Me.txtValue.Enabled = True And _
IsNull(Me.txtValue) _
Then
strInvalidDataMsg = "Enter a value!"
Set ctlInvalidEntry = Me.txtValue

ElseIf Me.txtMissions.Enabled = True And _
IsNull(Me.txtMissions) _
Then
strInvalidDataMsg = "Enter number of missions!"
Set ctlInvalidEntry = Me.txtMissions.SetFocus

End If

IsDataValid = blnResultOk
End Function

Private Function GetPrimaryKey() As Long
Dim lngPk As Long

Select Case Forms!frmsetup.subExplain.Form.txtAddEdit
Case "add"
strSql = fInsert Else strSql = fUpdate
Case "edit"
lngPk = Forms!frmsetup.subExplain.Form.ACT_PK
Case Else
lngPk = DMax("ACT_PK", "TBL_ACTIONS", _
"ACT_GAM_FK = " & Forms!frmsetup.txtPK)
End Select

GetPrimaryKey = lngPk
End Function

Private Sub ResyncSubExplainBookmark(lngPk As Long)
Dim rst As DAO.Recordset

Set rst = Forms!frmsetup.subExplain.Form.RecordsetClone
With rst
.MoveFirst
.FindFirst "ACT_PK = " & lngPk
Forms!frmsetup.subExplain.Form.Bookmark = .Bookmark
.MoveFirst
' Don't .Close it if you didn't .Open it
'.Close < Don't do this.
End With

Set rst = Nothing
End Sub

re: Use of Goto Exit_Proc

Oops - that's missing a piece...

Should be ...

If Not IsDataValid(strInvalidDataMsg, ctlInvalidEntry) Then
If Len(strInvalidDataMsg) > 0 then
Msgbox strInvalidDataMsg
End If
If Not ctlInvalidEntry Is Nothing Then
ctlInvalidEntry.SetFocus
End If

Exit Sub

End If



On Mon, 21 Feb 2005 12:07:14 -0800, Steve Jorgensen <nospam@nospam.nospam>
wrote:
[color=blue]
>On Mon, 21 Feb 2005 12:38:27 -0330, Tim Marshall
><TIMMY!@PurplePandaChasers.Moertherium> wrote:
>
>....[color=green]
>>Seriously though, how would you do it?[/color]
>
>The following is not tested, but it should give you the idea...
>
>Private Sub btnOk_Click()
>
>'This will run an insert/update for the main TBL_ACTION and a
>'series of INSERTS for TBL_ACTION_TERRAIN if this is a new
>'record. If an old record, a delete of existing
>'TBL_ACTION_TERRAIN will occur first
>
> Dim lngPk As Long
> Dim strInvalidDataMsg As String
> Dim ctlInvalidEntry As Access.Control
>
> On Error GoTo Err_Proc
>
> If Not IsDataValid(strInvalidDataMsg, ctlInvalidEntry) Then
> Exit Sub
> End If
>
> lngPk = GetPrimaryKey()
> fRestrictions lngPk
> Forms!frmsetup.subExplain.Form.Requery
> ResyncSubExplainBookmark lngPk
> DoCmd.Close acForm, "frmsetupActionsadd", acSaveNo
>
>Exit_Proc:
> Exit Sub
>
>Err_Proc:
> Select Case Err.Number
> Case 3022 'index - in this case, a repaeated name
> MsgBox "You already have an action with the " & _
> "abbreviation """ & Me.txtAbrev & """", _
> vbExclamation
> Me.txtName.SetFocus
>
> Case Else
> MsgBox "Error " & Err.Number & " " & Err.Description, _
> vbCritical, _
> "frmSetUpActionsAdd btnOk_Click", _
> Err.HelpFile, Err.HelpContext
>
> End Select
>
> Resume Exit_Proc
>
>End Sub
>
>Private Function IsBlankEntry(varEntry As Variant) As Boolean
> IsBlankEntry = (Len(Trim(varEntry & ""))=0)
>End Function
>
>Private Function IsDataValid( _
> ByRef strInvalidDataMsg As String, _
> ByRef ctlInvalidEntry As Access.Control _
>) As Boolean
> Dim blnResultOk As Boolean
>
> Dim intC As Integer
> Dim lngPk As Long
>
> If IsBlankEntry(Me.txtAbrev) Then
> blnResultOk = False
> strInvalidDataMsg = "Enter a unique (for this campaign" & _
> ") abbreviation/code for this action!"
> Set ctlInvalidEntry = Me.txtAbrev
>
> ElseIf IsBlankEntry(Me.txtName) Then
> blnResultOk = False
> strInvalidDataMsg = "Enter a name for this action!"
> Set ctlInvalidEntry = Me.txtName
>
> ElseIf Nz(Me.fraType, 0) = 0 Then
> blnResultOk = False
> strInvalidDataMsg = "You haven't chosen an action type."
> Set ctlInvalidEntry = Me.fraType
>
> ElseIf fCheckDest = False Then
> blnResultOk = False 'a separate procedure
>
> ElseIf fSelectTerrain = False
> blnResultOk = False
>
> ElseIf Me.fraType = 11 Or Me.fraType = 12 Then
>
> lngPk = 0
> For intC = 0 To Me.lstDestination.ListCount - 1
> If Me.lstDestination.Selected(intC) Then
> lngPk = Me.lstDestination.Column(0, intC)
> Exit For
> End If
> Next intC
>
> If lngPk = 0 Then 'nothing chosen
> blnResultOk = False
> strInvalidDataMsg = "Choose a terrain type ""effect""" & _
> " for the crossing/obstacle!"
> Set ctlInvalidEntry = Me.lstDestination
> End If
>
> ElseIf Me.txtValue.Enabled = True And _
> IsNull(Me.txtValue) _
> Then
> strInvalidDataMsg = "Enter a value!"
> Set ctlInvalidEntry = Me.txtValue
>
> ElseIf Me.txtMissions.Enabled = True And _
> IsNull(Me.txtMissions) _
> Then
> strInvalidDataMsg = "Enter number of missions!"
> Set ctlInvalidEntry = Me.txtMissions.SetFocus
>
> End If
>
> IsDataValid = blnResultOk
>End Function
>
>Private Function GetPrimaryKey() As Long
> Dim lngPk As Long
>
> Select Case Forms!frmsetup.subExplain.Form.txtAddEdit
> Case "add"
> strSql = fInsert Else strSql = fUpdate
> Case "edit"
> lngPk = Forms!frmsetup.subExplain.Form.ACT_PK
> Case Else
> lngPk = DMax("ACT_PK", "TBL_ACTIONS", _
> "ACT_GAM_FK = " & Forms!frmsetup.txtPK)
> End Select
>
> GetPrimaryKey = lngPk
>End Function
>
>Private Sub ResyncSubExplainBookmark(lngPk As Long)
> Dim rst As DAO.Recordset
>
> Set rst = Forms!frmsetup.subExplain.Form.RecordsetClone
> With rst
> .MoveFirst
> .FindFirst "ACT_PK = " & lngPk
> Forms!frmsetup.subExplain.Form.Bookmark = .Bookmark
> .MoveFirst
> ' Don't .Close it if you didn't .Open it
> '.Close < Don't do this.
> End With
>
> Set rst = Nothing
>End Sub[/color]

re: Use of Goto Exit_Proc

Tim Marshall <TIMMY!@PurplePandaChasers.Moertherium> wrote in
news:cvd11i$vg6$1@coranto.ucs.mun.ca:
[color=blue]
> Here's a "real life" proc. It's the OK button for a form that
> creates a new record. The table for the insert action has plenty
> and plenty of database level constraints, plus there are some
> other constarints I felt could best be handled at the form level.[/color]

Don't enable the OK button until all the conditions are met. You can
then use the AfterUpdate event of all the controls that have
required values to call a function that checks all the controls
involved and then enables the OK button.

It's probably a good idea in that case to use some kind of visual
indicator to show which fields are required, and, perhaps some kind
of "help" text that reads "all required fields (outlined in red)
must be completed to save the record."

This puts the validation code in the controls where the values are
edited, which is where I prefer for it to be, and informs the user
of invalid values as soon as they enter them, rather than waiting to
the end. Your code could result in a user clicking OK repeatedly,
each time getting a different error message, whereas my structure
tells them what to do *before* they click the OK button.

It also means the code behind the OK button doesn't need to do much
at all, and needs no GoTo's.

--
David W. Fenton http://www.bway.net/~dfenton
dfenton at bway dot net http://www.bway.net/~dfassoc

re: Use of Goto Exit_Proc

Steve Jorgensen <nospam@nospam.nospam> wrote in
news:k77k11p76aqgab4ro39f4r97dbqd1q95qj@4ax.com:
[color=blue]
> On Mon, 21 Feb 2005 12:43:18 -0330, Tim Marshall
><TIMMY!@PurplePandaChasers.Moertherium> wrote:
>[color=green]
>>Steve Jorgensen wrote:
>>[color=darkred]
>>> The use of Goto has lead to such awful spaghetti in the past
>>> that common wisdom has been to avoid using it - period.[/color]
>>
>>Thanks teve - could I ask you to have a look at my response to
>>Terry Kreft in this thread and offer an opinion? I think, though,
>>that your check list answered my question... 8)[/color]
>
> I could, but it will probably come down to 2 smart people who
> simply disagree. The only thing I might add is to say that I make
> my choices pragmatically, based on what will be easiest to read
> and maintain. I use certain things like the use of GoTo as red
> flags to tell me I might be on the wrong track, but with the
> emphasis on "might".[/color]

I would say that, discounting "On Error GoTo ErrHandler", if you
have more than one GoTo in your subroutine, you should consider
restructuring it.

After my .sig I've appended a piece of code I use in all my
applications. Can it be restructured to omit the GoTos into the
sub-subroutines? I don't think so, but maybe I'm missing something.

--
David W. Fenton http://www.bway.net/~dfenton
dfenton at bway dot net http://www.bway.net/~dfassoc

Public Function dbLocal(Optional ysnInitialize As Boolean = True) As
DAO.Database ' 2003/02/08 DWF added comments to explain it to
myself! ' uses GoTos instead of If/Then because:
' error of dbCurrent not being Nothing but dbCurrent being closed
' would (3420) would then be jumping back into the middle of an
' If/Then statement
On Error GoTo errHandler
Dim strTest As String

If Not ysnInitialize Then GoTo closeDB

retryDB:
If dbCurrent Is Nothing Then
Set dbCurrent = CurrentDb()
End If
' now that we know the db variable is not Nothing,
' test if it's Open
strTest = dbCurrent.Name

exitRoutine:
Set dbLocal = dbCurrent
Exit Function

closeDB:
If Not (dbCurrent Is Nothing) Then
'dbCurrent.close
Set dbCurrent = Nothing
End If
GoTo exitRoutine

errHandler:
Select Case Err.Number
Case 3420 ' Object invalid or no longer set.
Set dbCurrent = Nothing
If ysnInitialize Then
Resume retryDB
Else
Resume closeDB
End If
Case Else
MsgBox Err.Number & ": " & Err.Description, vbExclamation, _
"Error in Global Code.dbLocal()"
Resume exitRoutine
End Select
End Function

re: Use of Goto Exit_Proc

Hi Tim,

You don't want suggestions for code-change, so skip this message... ;-)

I validate field-input on the before_update event of the field, but I also need
record-validation at the form-level.
Nowadays I *always* put all my form-level validation (fields that must have a value) in a
separate function.
If you would do the same then IMO the readability of your code would improve much.
Another advantage: This function can be used elsewhere on your form !
e.g. in the Before_Update event, or when a user clicks a print-button (situations where
the record is saved, or must be saved)

Private Function RecordValidationFailed() as Boolean
Dim strmsg as string
Dim fError as boolean
strmsg = "This record fails validation... Please fill in:" & VbNewLine
'Now check all your fields
If IsNull(Field1) Then
strmsg=strmsg & " - Field 1" & vbNewLine
fError=True
end if
If IsNull(Field2) Then
strmsg=strmsg & " - Field 2" & vbNewLine
fError=True
end if
If IsNull(Field3) Then
strmsg=strmsg & " - Field 3" & vbNewLine
fError=True
end if
.. . . . . . .'and so on
If fError Then
Msgbox strmsg, VbExclamation, "Validation"
RecordValidationFailed=True
end if
End function

As you see this function 'does' basically the same as your code, but this function can be
used now in a generic way for this form.
A simular function can be used (modified for the fields) on *all* your other input-forms.

So now you could use:
If not RecordValidationFailed() Then
'Do your thing here
Else
Screen.PreviousControl.setfocus
End if


--
Hope this helps
Arno R



"Tim Marshall" <TIMMY!@PurplePandaChasers.Moertherium> schreef in bericht
news:cvd9hb$vg8$1@coranto.ucs.mun.ca...[color=blue]
> Tim Marshall wrote:
>[color=green]
>> Sorry for the change of topic, but I think I'm having a catharsis of sorts here...
>>
>> Terry Kreft wrote:[color=darkred]
>>> 3) Consider the ridicule of your peers when they see what you have done.[/color][/color]
>
> OK, rather than ask people to suggest code changes, perhaps Terry or someone else can
> tell me if the following is OK. Personally, I don't see a problem with the goto
> exit_proc approach... but I must avoid ridicule!!! 8)
>
> booProceed and strProceed are new variables here.
>
> Private Sub btnOk_Click()
>
> 'This will run an insert/update for the main TBL_ACTION and a series of INSERTS for
> 'TBL_ACTION_TERRAIN if this is a new record. If an old record, a delete of
> 'existing TBL_ACTION_TERRAIN will occur first
>
> Dim strSql As String
> Dim intC As Integer
> Dim lngPk As Long
> Dim rst As DAO.Recordset
>
> Dim booProceed As Boolean
> Dim strProceed As String
>
> On Error GoTo Err_Proc
>
> booProceed = True 'if any of the following make it false, procedure will not go
> ahead
> strProceed = ""
>
> 'ensure abbreviation and name is chosen
>
> If Nz(Me.txtAbrev, "") = "" Then
>
> strProceed = "Enter a unique (for this campaign) abbreviation/code for this
> action!"
>
> Me.txtAbrev.SetFocus
>
> booProceed = False
>
> End If
>
> 'ensure a name is selected for this action
>
> If Trim(Nz(Me.txtName, "")) = "" Then
>
> strProceed = strProceed & IIf(strProceed <> "", vbCrLf & vbCrLf, "") & "Enter a
> name for this action!"
>
> Me.txtName.SetFocus
>
> booProceed = False
>
> End If
>
> 'make sure an action type is selected
>
> If IsNull(Me.fraType) Or Me.fraType = 0 Then
>
> strProceed = strProceed & IIf(strProceed <> "", vbCrLf & vbCrLf, "") & "You
> haven't chosen an action type."
>
> Me.fraType.SetFocus
>
> booProceed = False
>
> End If
>
> 'next verify that if 11 or 12 that one or all destination list box item is selected
>
> If fCheckDest = False Then booProceed = False
>
> If fSelectTerrain = False Then booProceed = False
>
> 'next make sure that only one value is selected
>
> If Me.fraType = 11 Or Me.fraType = 12 Then
>
> lngPk = 0
>
> For intC = 0 To Me.lstDestination.ListCount - 1
>
> If Me.lstDestination.Selected(intC) Then
>
> lngPk = Me.lstDestination.Column(0, intC)
>
> Exit For
>
> End If
>
> Next intC
>
> If lngPk = 0 Then 'nothing chosen
>
> strProceed = strProceed & IIf(strProceed <> "", vbCrLf & vbCrLf, "") &
> "Choose a terrain type ""effect"" for the crossing/obstacle!"
>
> Me.lstDestination.SetFocus
>
> booProceed = False
>
> End If
>
> End If
>
> 'is there a
>
> 'verify that there is a value in txtValue. if it is enabled, there needs to be a
> value
>
> If Me.txtValue.Enabled = True Then
>
> If IsNull(Me.txtValue) Then
>
> strProceed = strProceed & IIf(strProceed <> "", vbCrLf & vbCrLf, "") &
> "Enter a value!"
>
> Me.txtValue.SetFocus
>
> booProceed = False
>
> End If
>
> End If
>
> 'missions
>
> If Me.txtMissions.Enabled = True Then
>
> If IsNull(Me.txtMissions) Then
>
> strProceed = strProceed & IIf(strProceed <> "", vbCrLf & vbCrLf, "") &
> "Enter number of missions!"
>
> Me.txtMissions.SetFocus
>
> booProceed = False
>
> End If
>
> End If
>
> If booProceed = False Then
>
> 'display error messsage if strProceed is not empty string, otherwise, the
> 'separate routines will have displayed a message
>
> If strProceed <> "" Then MsgBox strProceed, vbExclamation, "Incomplete
> Information for This Action"
>
> Else
>
> 'do the action in tbl_action first and get the pk
>
> If Forms!frmsetup.subExplain.Form.txtAddEdit = "add" Then strSql = fInsert Else
> strSql = fUpdate
>
> 'Find lngPK, ACT_PK of new/updated action
>
> If Forms!frmsetup.subExplain.Form.txtAddEdit = "edit" Then
>
> lngPk = Forms!frmsetup.subExplain.Form.ACT_PK
>
> Else
>
> 'just do a dmax, that will be the latest one
>
> lngPk = DMax("ACT_PK", "TBL_ACTIONS", "ACT_GAM_FK = " &
> Forms!frmsetup.txtPK)
>
> End If
>
> 'Now do terrain restrictions
>
> fRestrictions lngPk
>
> 'requery
>
> Forms!frmsetup.subExplain.Form.Requery
>
> 'bookmark
>
> Set rst = Forms!frmsetup.subExplain.Form.RecordsetClone
>
> With rst
>
> .MoveFirst
>
> .FindFirst "ACT_PK = " & lngPk
>
> Forms!frmsetup.subExplain.Form.Bookmark = .Bookmark
>
> .MoveFirst
>
> .Close
>
> End With
>
> DoCmd.Close acForm, "frmsetupActionsadd", acSaveNo
>
> End If
>
> Exit_Proc:
>
> Set rst = Nothing
>
> Exit Sub
>
> Err_Proc:
>
> Select Case Err.Number
>
> Case 3022 'index - in this case, a repaeated name
>
> MsgBox "You already have an action with the abbreviation """ & Me.txtAbrev &
> """", vbExclamation
>
> Me.txtName.SetFocus
>
> GoTo Exit_Proc
>
> Case Else
>
> MsgBox "Error " & Err.Number & " " & Err.Description, vbCritical,
> "frmSetUpActionsAdd btnOk_Click", Err.HelpFile, Err.HelpContext
>
> GoTo Exit_Proc
>
> End Select
>
>
> End Sub
>
>
> --
> Tim http://www.ucs.mun.ca/~tmarshal/
> ^o<
> /#) "Burp-beep, burp-beep, burp-beep?" - Quaker Jake
> /^^ "What's UP, Dittoooooo?" - Ditto[/color]

re: Use of Goto Exit_Proc

Tim Marshall <TIMMY!@PurplePandaChasers.Moertherium> wrote:
[color=blue]
>I used to have separate exit function/sub along with whatever closing of
>variables. I began using GOTo as a means to end the procedure
>prematurely if certain conditions are not meant.[/color]

I do exactly the same thing myself.

Tony
--
Tony Toews, Microsoft Access MVP
Please respond only in the newsgroups so that others can
read the entire thread of messages.
Microsoft Access Links, Hints, Tips & Accounting Systems at
http://www.granite.ab.ca/accsmstr.htm

re: Use of Goto Exit_Proc

On Mon, 21 Feb 2005 20:53:29 GMT, "David W. Fenton"
<dXXXfenton@bway.net.invalid> wrote:

....

(Putting stuff after the Sig line turns out to mess up Forte Agent because it
does not quote the text in the reply).

IMO, your code is about as good as it can get. The only things I can see that
would get rid of the Goto involve adding duplication, and adding duplication
is way worse than employing a Goto.


Copy of code in message replied to:

Public Function dbLocal(Optional ysnInitialize As Boolean = True) As
DAO.Database ' 2003/02/08 DWF added comments to explain it to
myself! ' uses GoTos instead of If/Then because:
' error of dbCurrent not being Nothing but dbCurrent being closed
' would (3420) would then be jumping back into the middle of an
' If/Then statement
On Error GoTo errHandler
Dim strTest As String

If Not ysnInitialize Then GoTo closeDB

retryDB:
If dbCurrent Is Nothing Then
Set dbCurrent = CurrentDb()
End If
' now that we know the db variable is not Nothing,
' test if it's Open
strTest = dbCurrent.Name

exitRoutine:
Set dbLocal = dbCurrent
Exit Function

closeDB:
If Not (dbCurrent Is Nothing) Then
'dbCurrent.close
Set dbCurrent = Nothing
End If
GoTo exitRoutine

errHandler:
Select Case Err.Number
Case 3420 ' Object invalid or no longer set.
Set dbCurrent = Nothing
If ysnInitialize Then
Resume retryDB
Else
Resume closeDB
End If
Case Else
MsgBox Err.Number & ": " & Err.Description, vbExclamation, _
"Error in Global Code.dbLocal()"
Resume exitRoutine
End Select
End Function

re: Use of Goto Exit_Proc

On Mon, 21 Feb 2005 21:33:43 GMT, Tony Toews <ttoews@telusplanet.net> wrote:
[color=blue]
>Tim Marshall <TIMMY!@PurplePandaChasers.Moertherium> wrote:
>[color=green]
>>I used to have separate exit function/sub along with whatever closing of
>>variables. I began using GOTo as a means to end the procedure
>>prematurely if certain conditions are not meant.[/color]
>
>I do exactly the same thing myself.[/color]

As do I - it's called a guard clause. The code presented, however, when
restructured in a clear way, no longer has a need for a guard clause employing
Goto. My example (for example) does include a guard clause, but it uses Exit
Sub.

re: Use of Goto Exit_Proc

On Mon, 21 Feb 2005 20:53:29 GMT, "David W. Fenton"
<dXXXfenton@bway.net.invalid> wrote:

....

(Putting stuff after the Sig line turns out to mess up Forte Agent because it
does not quote the text in the reply).

IMO, your code is about as good as it can get. The only things I can see that
would get rid of the Goto involve adding duplication, and adding duplication
is way worse than employing a Goto.


Copy of code in message replied to:

Public Function dbLocal(Optional ysnInitialize As Boolean = True) As
DAO.Database ' 2003/02/08 DWF added comments to explain it to
myself! ' uses GoTos instead of If/Then because:
' error of dbCurrent not being Nothing but dbCurrent being closed
' would (3420) would then be jumping back into the middle of an
' If/Then statement
On Error GoTo errHandler
Dim strTest As String

If Not ysnInitialize Then GoTo closeDB

retryDB:
If dbCurrent Is Nothing Then
Set dbCurrent = CurrentDb()
End If
' now that we know the db variable is not Nothing,
' test if it's Open
strTest = dbCurrent.Name

exitRoutine:
Set dbLocal = dbCurrent
Exit Function

closeDB:
If Not (dbCurrent Is Nothing) Then
'dbCurrent.close
Set dbCurrent = Nothing
End If
GoTo exitRoutine

errHandler:
Select Case Err.Number
Case 3420 ' Object invalid or no longer set.
Set dbCurrent = Nothing
If ysnInitialize Then
Resume retryDB
Else
Resume closeDB
End If
Case Else
MsgBox Err.Number & ": " & Err.Description, vbExclamation, _
"Error in Global Code.dbLocal()"
Resume exitRoutine
End Select
End Function

re: Use of Goto Exit_Proc

On Mon, 21 Feb 2005 21:33:43 GMT, Tony Toews <ttoews@telusplanet.net> wrote:
[color=blue]
>Tim Marshall <TIMMY!@PurplePandaChasers.Moertherium> wrote:
>[color=green]
>>I used to have separate exit function/sub along with whatever closing of
>>variables. I began using GOTo as a means to end the procedure
>>prematurely if certain conditions are not meant.[/color]
>
>I do exactly the same thing myself.[/color]

As do I - it's called a guard clause. The code presented, however, when
restructured in a clear way, no longer has a need for a guard clause employing
Goto. My example (for example) does include a guard clause, but it uses Exit
Sub.

re: Use of Goto Exit_Proc

Steve Jorgensen <nospam@nospam.nospam> wrote:
[color=blue][color=green][color=darkred]
>>>I used to have separate exit function/sub along with whatever closing of
>>>variables. I began using GOTo as a means to end the procedure
>>>prematurely if certain conditions are not meant.[/color]
>>
>>I do exactly the same thing myself.[/color]
>
>As do I - it's called a guard clause. The code presented, however, when
>restructured in a clear way, no longer has a need for a guard clause employing
>Goto. My example (for example) does include a guard clause, but it uses Exit
>Sub.[/color]

Agreed that there are some interesting ideas which I will certainly consider for the
future.

Tony
--
Tony Toews, Microsoft Access MVP
Please respond only in the newsgroups so that others can
read the entire thread of messages.
Microsoft Access Links, Hints, Tips & Accounting Systems at
http://www.granite.ab.ca/accsmstr.htm

re: Use of Goto Exit_Proc

Steve Jorgensen <nospam@nospam.nospam> wrote:
[color=blue][color=green][color=darkred]
>>>I used to have separate exit function/sub along with whatever closing of
>>>variables. I began using GOTo as a means to end the procedure
>>>prematurely if certain conditions are not meant.[/color]
>>
>>I do exactly the same thing myself.[/color]
>
>As do I - it's called a guard clause. The code presented, however, when
>restructured in a clear way, no longer has a need for a guard clause employing
>Goto. My example (for example) does include a guard clause, but it uses Exit
>Sub.[/color]

Agreed that there are some interesting ideas which I will certainly consider for the
future.

Tony
--
Tony Toews, Microsoft Access MVP
Please respond only in the newsgroups so that others can
read the entire thread of messages.
Microsoft Access Links, Hints, Tips & Accounting Systems at
http://www.granite.ab.ca/accsmstr.htm

re: Use of Goto Exit_Proc

Steve Jorgensen <nospam@nospam.nospam> wrote in
news:63nk1192gmfn5cbfrjcdd7j4i9nvglh68q@4ax.com:
[color=blue]
> On Mon, 21 Feb 2005 20:53:29 GMT, "David W. Fenton"
><dXXXfenton@bway.net.invalid> wrote:
>
> ...
>
> (Putting stuff after the Sig line turns out to mess up Forte Agent
> because it does not quote the text in the reply).[/color]

xNews quotes nothing after the sig marker unless you select the
whole message before initiating the followup. I always assume that
people are using tools that are just as good as mine. ;)
[color=blue]
> IMO, your code is about as good as it can get. The only things I
> can see that would get rid of the Goto involve adding duplication,
> and adding duplication is way worse than employing a Goto.[/color]

You know, looking at it again, there aren't as many GoTo's as I
thought -- I think I was consider the Resume statements in the error
handler as GoTo's, which is kind of what they are, actually (a GoTo
that will work during an error condition). They have the same bad
effect as GoTo's, in that they multiply the number of possible paths
through the code.

--
David W. Fenton http://www.bway.net/~dfenton
dfenton at bway dot net http://www.bway.net/~dfassoc

re: Use of Goto Exit_Proc

Steve Jorgensen <nospam@nospam.nospam> wrote in
news:63nk1192gmfn5cbfrjcdd7j4i9nvglh68q@4ax.com:
[color=blue]
> On Mon, 21 Feb 2005 20:53:29 GMT, "David W. Fenton"
><dXXXfenton@bway.net.invalid> wrote:
>
> ...
>
> (Putting stuff after the Sig line turns out to mess up Forte Agent
> because it does not quote the text in the reply).[/color]

xNews quotes nothing after the sig marker unless you select the
whole message before initiating the followup. I always assume that
people are using tools that are just as good as mine. ;)
[color=blue]
> IMO, your code is about as good as it can get. The only things I
> can see that would get rid of the Goto involve adding duplication,
> and adding duplication is way worse than employing a Goto.[/color]

You know, looking at it again, there aren't as many GoTo's as I
thought -- I think I was consider the Resume statements in the error
handler as GoTo's, which is kind of what they are, actually (a GoTo
that will work during an error condition). They have the same bad
effect as GoTo's, in that they multiply the number of possible paths
through the code.

--
David W. Fenton http://www.bway.net/~dfenton
dfenton at bway dot net http://www.bway.net/~dfassoc

re: Use of Goto Exit_Proc

David W. Fenton wrote:[color=blue]
> xNews quotes nothing after the sig marker unless you select the
> whole message before initiating the followup. I always assume that
> people are using tools that are just as good as mine. ;)[/color]

ISTR Agent does that as well.

--
This sig left intentionally blank

re: Use of Goto Exit_Proc

David,
Maybe I'm missing something but isn't your code functionally the same as

If ysnInitialize Then
If dbCurrent Is Nothing Then
Set dbCurrent = CurrentDb()
End If
Set dbLocal = dbCurrent
Else
Set dbCurrent = Nothing
End If


--
Terry Kreft
MVP Microsoft Access


"David W. Fenton" <dXXXfenton@bway.net.invalid> wrote in message
news:Xns9604A1D2AE132dfentonbwaynetinvali@24.168.1 28.86...[color=blue]
> Steve Jorgensen <nospam@nospam.nospam> wrote in
> news:k77k11p76aqgab4ro39f4r97dbqd1q95qj@4ax.com:
>[color=green]
> > On Mon, 21 Feb 2005 12:43:18 -0330, Tim Marshall
> ><TIMMY!@PurplePandaChasers.Moertherium> wrote:
> >[color=darkred]
> >>Steve Jorgensen wrote:
> >>
> >>> The use of Goto has lead to such awful spaghetti in the past
> >>> that common wisdom has been to avoid using it - period.
> >>
> >>Thanks teve - could I ask you to have a look at my response to
> >>Terry Kreft in this thread and offer an opinion? I think, though,
> >>that your check list answered my question... 8)[/color]
> >
> > I could, but it will probably come down to 2 smart people who
> > simply disagree. The only thing I might add is to say that I make
> > my choices pragmatically, based on what will be easiest to read
> > and maintain. I use certain things like the use of GoTo as red
> > flags to tell me I might be on the wrong track, but with the
> > emphasis on "might".[/color]
>
> I would say that, discounting "On Error GoTo ErrHandler", if you
> have more than one GoTo in your subroutine, you should consider
> restructuring it.
>
> After my .sig I've appended a piece of code I use in all my
> applications. Can it be restructured to omit the GoTos into the
> sub-subroutines? I don't think so, but maybe I'm missing something.
>
> --
> David W. Fenton http://www.bway.net/~dfenton
> dfenton at bway dot net http://www.bway.net/~dfassoc
>
> Public Function dbLocal(Optional ysnInitialize As Boolean = True) As
> DAO.Database ' 2003/02/08 DWF added comments to explain it to
> myself! ' uses GoTos instead of If/Then because:
> ' error of dbCurrent not being Nothing but dbCurrent being closed
> ' would (3420) would then be jumping back into the middle of an
> ' If/Then statement
> On Error GoTo errHandler
> Dim strTest As String
>
> If Not ysnInitialize Then GoTo closeDB
>
> retryDB:
> If dbCurrent Is Nothing Then
> Set dbCurrent = CurrentDb()
> End If
> ' now that we know the db variable is not Nothing,
> ' test if it's Open
> strTest = dbCurrent.Name
>
> exitRoutine:
> Set dbLocal = dbCurrent
> Exit Function
>
> closeDB:
> If Not (dbCurrent Is Nothing) Then
> 'dbCurrent.close
> Set dbCurrent = Nothing
> End If
> GoTo exitRoutine
>
> errHandler:
> Select Case Err.Number
> Case 3420 ' Object invalid or no longer set.
> Set dbCurrent = Nothing
> If ysnInitialize Then
> Resume retryDB
> Else
> Resume closeDB
> End If
> Case Else
> MsgBox Err.Number & ": " & Err.Description, vbExclamation, _
> "Error in Global Code.dbLocal()"
> Resume exitRoutine
> End Select
> End Function[/color]

re: Use of Goto Exit_Proc

"Terry Kreft" <terry.kreft@mps.co.uk> wrote in
news:vZScnerN-8elYIffSa8jmA@karoo.co.uk:
[color=blue]
> Maybe I'm missing something but isn't your code functionally the
> same as
>
> If ysnInitialize Then
> If dbCurrent Is Nothing Then
> Set dbCurrent = CurrentDb()
> End If
> Set dbLocal = dbCurrent
> Else
> Set dbCurrent = Nothing
> End If[/color]

No, because dbCurrent can be Nothing while the db it points to is
closed. I don't know how it happens with the CurrentDB() (perhaps
the wizard problem?), but it happened to me, so I wrote the code to
avoid it. If you can come up with a simpler way to handle that
condition, then I'm all for it.

This code did, in fact, start out very simple, not unlike what you
suggest, and it only got complicated after I was using it for a long
enough time to encounter a number of errors that weren't obvious to
me at first.

--
David W. Fenton http://www.bway.net/~dfenton
dfenton at bway dot net http://www.bway.net/~dfassoc

re: Use of Goto Exit_Proc

Arno R wrote:
[color=blue]
> Hi Tim,
>
> You don't want suggestions for code-change, so skip this message... ;-)[/color]

Not at all, I appeciate any input or suggestions...
[color=blue]
> I validate field-input on the before_update event of the field,[/color]

I must say I do this too, though if my field validation involves
changing the value, I go with the after update event since I have found
a lot of difficulty manipulating the .text value.
[color=blue]
> but I also need
> record-validation at the form-level.
> Nowadays I *always* put all my form-level validation (fields that must have a value) in a
> separate function.
> If you would do the same then IMO the readability of your code would improve much.[/color]

Good idea. I suspect I sometimes pack too much into my procedures.
[color=blue]
> Another advantage: This function can be used elsewhere on your form ![/color]


Thanks I really appreciate your time, Arno.


--
Tim http://www.ucs.mun.ca/~tmarshal/
^o<
/#) "Burp-beep, burp-beep, burp-beep?" - Quaker Jake
/^^ "What's UP, Dittoooooo?" - Ditto

re: Use of Goto Exit_Proc

Bri wrote:
[color=blue]
> Tim,
>
> Your code does allow for a lot of confusion and maintenance problems
> down the line. Also, it can frustrate your users as they fix one problem
> and try again only to get yet another new error message. Here is
> something I've used that might help.[/color]

Thanks. I think perhaps the main thing to d is break up the code. I do
this sometimes and othertimes I don't. I should do it mor often.

--
Tim http://www.ucs.mun.ca/~tmarshal/
^o<
/#) "Burp-beep, burp-beep, burp-beep?" - Quaker Jake
/^^ "What's UP, Dittoooooo?" - Ditto

re: Use of Goto Exit_Proc

David W. Fenton wrote:
[color=blue]
> Don't enable the OK button until all the conditions are met. You can
> then use the AfterUpdate event of all the controls that have
> required values to call a function that checks all the controls
> involved and then enables the OK button.[/color]

<snip>

Thanks very much David. I've done this before and should try to
remember it. I think the message I'm getting from all and sundry 8) is
to a combination of breaking up the code into sub routines and/or field
level validation.

I appreciate your time, thanks. 8)
--
Tim http://www.ucs.mun.ca/~tmarshal/
^o<
/#) "Burp-beep, burp-beep, burp-beep?" - Quaker Jake
/^^ "What's UP, Dittoooooo?" - Ditto

re: Use of Goto Exit_Proc

On Mon, 21 Feb 2005 20:53:29 GMT, "David W. Fenton"
<dXXXfenton@bway.net.invalid> wrote:

....

I thought about your code module again, and realized that it could probably be
clarified by extracting a test for whether the database is open into its own
procedure, like the example below.

Of course, I'm a fan of using an explicit variable to hold a return value,
even if it means I have to use a Goto as a return, but using the function name
as an implicit variable, the goto could be replaced with Exit Function if you
wanted to.

== Proposed Code ==

Public Function dbLocal( _
Optional ysnInitialize As Boolean = True _
) As DAO.Database
On Error GoTo err_catch
Dim blnIsDbOpen As Boolean

blnIsDbOpen = IsDbOpen(dbCurrent)

If ysnInitialize Then GoTo closeDB Then
If Not blnIsDbOpen Then Set dbCurrent = CurrentDb()
Else
If blnIsDbOpen Then dbCurrent.Close
Set dbCurrent = Nothing
End If

Fn_Return:
Set dbLocal = dbCurrent
Exit Function

err_catch:
MsgBox Err.Number & ": " & Err.Description, vbExclamation, _
"Error in Global Code.dbLocal()"
Resume Fn_Return

End Function

' Calling procedure is expected to handle any unexpected errors.
Public Function IsDbOpen(dbToCheck As DAO.Database) As Boolean
Dim strNameTest As String
Dim blnResult As Boolean

blnResult = True ' Assume True unless we find out otherwise.

If dbToCheck Is Nothing Then
blnResult = False: Goto Fn_Return
End If

On Error Goto Err_Catch
strNameTest = dbToCheck.Name

Fn_Return:
IsDbOpen = blnResult

Err_Catch:
Select Case Err.Number
Case 3420 ' Object invalid or no longer set.
blnResult = False
Resume Fn_Return
Case Else
Err.Raise Err.Number, Err.Source, Err.Description,
Err.HelpFile, Err.HelpContext
End Select

End Function


== Code from original post ==

Public Function dbLocal(Optional ysnInitialize As Boolean = True) As
DAO.Database ' 2003/02/08 DWF added comments to explain it to
myself! ' uses GoTos instead of If/Then because:
' error of dbCurrent not being Nothing but dbCurrent being closed
' would (3420) would then be jumping back into the middle of an
' If/Then statement
On Error GoTo errHandler
Dim strTest As String

If Not ysnInitialize Then GoTo closeDB

retryDB:
If dbCurrent Is Nothing Then
Set dbCurrent = CurrentDb()
End If
' now that we know the db variable is not Nothing,
' test if it's Open
strTest = dbCurrent.Name

exitRoutine:
Set dbLocal = dbCurrent
Exit Function

closeDB:
If Not (dbCurrent Is Nothing) Then
'dbCurrent.close
Set dbCurrent = Nothing
End If
GoTo exitRoutine

errHandler:
Select Case Err.Number
Case 3420 ' Object invalid or no longer set.
Set dbCurrent = Nothing
If ysnInitialize Then
Resume retryDB
Else
Resume closeDB
End If
Case Else
MsgBox Err.Number & ": " & Err.Description, vbExclamation, _
"Error in Global Code.dbLocal()"
Resume exitRoutine
End Select
End Function

re: Use of Goto Exit_Proc

"Terry Kreft" wrote
[color=blue]
> I've been coding for 13 years in VB and
> VBA and can think of only one occasion
> where I have used GoTo except as part
> of an error handling routine.
>
> The example you show does not require GoTo at all.
>
> My personal advice is; if you feel the need to use goto:
> 1) Try calmimg down, you are obviously
> overworked and possibly a bit
> emotional.
> 2) Have a cold shower
> 3) Consider the ridicule of your peers when
> they see what you have done. <G>[/color]

I was one of IBM's internal advocates for structured programming, back when
it was a "new thing" (a long time ago -- a little over 30 years ago, I was
invited to be part of a team to take a rotational assignment in Europe to
promote structured programming in IBM World Trade, but I opted to take an
assignment in San Francisco instead, and do not regret having done so) and
my view was, and is, this: When one "converts" from spaghetti code to
structured, it is useful to be strict with yourself for a period of time and
allow yourself no violations; then (as Steve said) _judicious_ violations
can make your code easier to read/follow, and that is the whole reason for
structured programming -- make it easier to read/follow and, thus, avoid
mistakes.

It is just as much a violation of structure to sprinkle Exit Sub/Function
statements through the code as it is to use Go To Exit_Proc. And, of course,
the Exit_Proc may contain other code besides the Exit Sub/Function (like the
"Finally" code of "Try / Catch / Finally" that the OOP advocates think is so
much better).

And, if I understood, the only case in which either Tim or Steve proposed
using Go To was _in conjunction with error handling_. If you do not, you are
very likely to find yourself including unnneccesary layers of conditions
just to avoid GoTos in case of error and clear need to "quit right now".
Unneccesary layers of conditions will make the code harder to read/follow
and be more conducive to the introduction of errors than a few GoTos.

Larry Linson
Microsoft Access MVP

re: Use of Goto Exit_Proc

On Thu, 24 Feb 2005 17:04:24 GMT, "Larry Linson" <bouncer@localhost.not>
wrote:
[color=blue]
>"Terry Kreft" wrote
>[color=green]
> > I've been coding for 13 years in VB and
> > VBA and can think of only one occasion
> > where I have used GoTo except as part
> > of an error handling routine.
> >
> > The example you show does not require GoTo at all.
> >
> > My personal advice is; if you feel the need to use goto:
> > 1) Try calmimg down, you are obviously
> > overworked and possibly a bit
> > emotional.
> > 2) Have a cold shower
> > 3) Consider the ridicule of your peers when
> > they see what you have done. <G>[/color]
>
>I was one of IBM's internal advocates for structured programming, back when
>it was a "new thing" (a long time ago -- a little over 30 years ago, I was
>invited to be part of a team to take a rotational assignment in Europe to
>promote structured programming in IBM World Trade, but I opted to take an
>assignment in San Francisco instead, and do not regret having done so) and
>my view was, and is, this: When one "converts" from spaghetti code to
>structured, it is useful to be strict with yourself for a period of time and
>allow yourself no violations; then (as Steve said) _judicious_ violations
>can make your code easier to read/follow, and that is the whole reason for
>structured programming -- make it easier to read/follow and, thus, avoid
>mistakes.
>
>It is just as much a violation of structure to sprinkle Exit Sub/Function
>statements through the code as it is to use Go To Exit_Proc. And, of course,
>the Exit_Proc may contain other code besides the Exit Sub/Function (like the
>"Finally" code of "Try / Catch / Finally" that the OOP advocates think is so
>much better).
>
>And, if I understood, the only case in which either Tim or Steve proposed
>using Go To was _in conjunction with error handling_. If you do not, you are
>very likely to find yourself including unnneccesary layers of conditions
>just to avoid GoTos in case of error and clear need to "quit right now".
>Unneccesary layers of conditions will make the code harder to read/follow
>and be more conducive to the introduction of errors than a few GoTos.
>
> Larry Linson
> Microsoft Access MVP
>[/color]

Actually, I also often advocate using Goto to jump to a clean-up/exit block,
even when no error handling code exists in the procedure.

re: Use of Goto Exit_Proc

Steve Jorgensen wrote:
[color=blue]
> Actually, I also often advocate using Goto to jump to a clean-up/exit[/color]
block,[color=blue]
> even when no error handling code exists in the procedure.[/color]

I think unconditional exclusion of GoTo's (space omitted) has served
it's purpose of eliminating the unorganized spaghetti code that
prevaled under BASIC and FORTRAN. GoTo was certainly good at making
code confusing and unstructured. That kind of coding is long gone (I
hope) so I won't malign any programmer who has a decent reason for
using it. I usually rewrite control structures to avoid GoTo since
it's nearly always possible to do that. In certain cases I guess I'd
even include not having the time to rewrite the control structure as a
valid reason for temporarily having a GoTo. The idea of using GoTo to
jump to a clean-up/exit block won't draw any criticism from me either.
Just allow me to decide how many times I need to duplicate clean-up
code before using it :-). You could even use a GoSub ... Return to run
the clean-up before exiting. Someone in this NG mentioned that it only
takes a few lines of code to allow a called Sub to access the variables
of the calling routine. I should have followed up on that.

James A. Fortune

People rationalize traffic laws. Police take them literally. Choose
wisely.

re: Use of Goto Exit_Proc

Steve Jorgensen <nospam@nospam.nospam> wrote in
news:jsur115sgcg5mlgslqbkr4e4rafp8r92be@4ax.com:
[color=blue]
> If ysnInitialize Then GoTo closeDB Then
> If Not blnIsDbOpen Then Set dbCurrent = CurrentDb()
> Else
> If blnIsDbOpen Then dbCurrent.Close
> Set dbCurrent = Nothing
> End If[/color]

The first line of your code block is nonsense.

--
David W. Fenton http://www.bway.net/~dfenton
dfenton at bway dot net http://www.bway.net/~dfassoc

re: Use of Goto Exit_Proc

Steve Jorgensen <nospam@nospam.nospam> wrote in
news:7u2s11l7irltdr5jl8t5fqflj79ldm6ua4@4ax.com:
[color=blue]
> Actually, I also often advocate using Goto to jump to a
> clean-up/exit block, even when no error handling code exists in
> the procedure.[/color]

A guard clause that is the first thing in a procedure doesn't need
to jump to the clean-up/exit block -- Exit Sub/Function is fine
there, since if the guard clause is the first thing, there are no
structures to clean up.

--
David W. Fenton http://www.bway.net/~dfenton
dfenton at bway dot net http://www.bway.net/~dfassoc

re: Use of Goto Exit_Proc

On Thu, 24 Feb 2005 19:50:35 GMT, "David W. Fenton"
<dXXXfenton@bway.net.invalid> wrote:
[color=blue]
>Steve Jorgensen <nospam@nospam.nospam> wrote in
>news:jsur115sgcg5mlgslqbkr4e4rafp8r92be@4ax.com :
>[color=green]
>> If ysnInitialize Then GoTo closeDB Then
>> If Not blnIsDbOpen Then Set dbCurrent = CurrentDb()
>> Else
>> If blnIsDbOpen Then dbCurrent.Close
>> Set dbCurrent = Nothing
>> End If[/color]
>
>The first line of your code block is nonsense.[/color]

Yes, it's a typo. it should read just "If ysnInitialize Then".

re: Use of Goto Exit_Proc

On Thu, 24 Feb 2005 19:53:14 GMT, "David W. Fenton"
<dXXXfenton@bway.net.invalid> wrote:
[color=blue]
>Steve Jorgensen <nospam@nospam.nospam> wrote in
>news:7u2s11l7irltdr5jl8t5fqflj79ldm6ua4@4ax.com :
>[color=green]
>> Actually, I also often advocate using Goto to jump to a
>> clean-up/exit block, even when no error handling code exists in
>> the procedure.[/color]
>
>A guard clause that is the first thing in a procedure doesn't need
>to jump to the clean-up/exit block -- Exit Sub/Function is fine
>there, since if the guard clause is the first thing, there are no
>structures to clean up.[/color]

Guard clauses are not always at the absolute top. Sometimes, they are
checking initial state conditions that cannot be determined prior to
initializing the objects necessary to check them. These objects must be torn
down. I do try to avoid the issue, but the cure for that is sometimes worse
than the disease.