Multiple Exit Sub's - A Bad Thing?

[Edward]

I have been handed this code to use in my app.

If (dataTable Is Nothing) Then
Exit Sub
End If
If (colStyle Is Nothing) Then
Exit Sub
End If

I would prefer:

Dim blnContinue As Boolean = True

If (dataTable Is Nothing) Then
blnContinue = False
End If
If (colStyle Is Nothing) Then
blnContinue = False
End If

If (blnContinue) Then
' Do my thang

Am I just being picky? I was taught that one should have a single
exit point in every function.

TIA

Edward

 

[EricJ]

what about
If (dataTable Is Nothing) OR (colStyle Is Nothing) Then
Exit Sub
End If

 

[Cor]

Hi Edward,
Why not just

If not dataTable Is Nothing Then
if not colStyle Is Nothing Then
' do your things
end if
end if

It is VB not a C# like language with brackets on the most strange places
)
Cor

 

[Fergus Cooney]

Hi Edward,

|| I was taught that one should have
|| a single exit point in every function.

You was taught wrong!

Here's the correct version.

One should have a single exit point in every function where it makes more
sense. And as many more as necessary when it makes sense.

No, actually you were taught correctly enough - for the stage you were at.
For a beginner programmer the absolute rule makes it easy to form good enough
habits. As an experienced programmer, you will be making your own choices -
breaking some rules and finding out that things are actually a bit better
(when you get it right).

It's impossible to teach the version that I gave because it just reeks of
the need for experience.

Personally I like to get my error checking over and done with at the top
of a routine. Once those checks are done, everything below can assume that the
parameters are valid (for example).

Having a Boolean which gets tested means not knowing until the very end
whether a bad parameter causes an exit or some extra processing somewhere
below. Or it's made clear by extra levels of indentation which does nothing
for the conservation of space (a valuable commodity).

Sub Foo (TheArg)
If ItsWrong (TheArg)
Exit Sub 'Nuff said.
End If
'From here onwards TheArg is good stuff...
: : :
'Plenty more but not too much.
: : :
End Sub

When you rigidly stick to the 'Use A Boolean and Keep Checking It' rule to
control program flow, you can sometimes get tied in knots as bad as GoTos used
to be - trying to fight your way out of the code to reach the bottom. In these
cases a simple Exit Sub will cut the crap quite nicely. ;-)

One example, is search loops. The value gets found in the middle of the
loop. All I want to do is return it. There's a wonderful keyword - Return -
created just for the job and I'm happy to use it. If we weren't supposed to
exit before the end, languages would never provide Returns and Exit Sub.

If there's concern that a subsequent programmer will overlook the abrupt
exit - document it in lovely big capitals at the top of your routine (or not).
Seriously, though, it's easy enough to list the terminating conditions of a
routine.

Are you being 'being picky'? Well, let's change the phrase to 'being
choosy'. And now, because you're asking the question, we change it to 'giving
the matter consideration'. All of a sudden it sounds like a good idea to me.
Something that indicates the development of maturity and your own style. )

Regards,
Fergus

 

[Edward]

Hi Edward,
Why not just

If not dataTable Is Nothing Then
if not colStyle Is Nothing Then
' do your things
end if
end if

That's good.

It is VB not a C# like language with brackets on the most strange places
)
Cor

Brackets help to remove ambiguity, and IMO aid readability and make
intended logic more clear.

Edward

 

[Edward]

Hi Edward,

|| I was taught that one should have
|| a single exit point in every function.

You was taught wrong!

Here's the correct version.

One should have a single exit point in every function where it makes more
sense. And as many more as necessary when it makes sense.

No, actually you were taught correctly enough - for the stage you were at.
For a beginner programmer the absolute rule makes it easy to form good enough
habits. As an experienced programmer, you will be making your own choices -
breaking some rules and finding out that things are actually a bit better
(when you get it right).

It's impossible to teach the version that I gave because it just reeks of
the need for experience.

Personally I like to get my error checking over and done with at the top
of a routine. Once those checks are done, everything below can assume that the
parameters are valid (for example).

Having a Boolean which gets tested means not knowing until the very end
whether a bad parameter causes an exit or some extra processing somewhere
below. Or it's made clear by extra levels of indentation which does nothing
for the conservation of space (a valuable commodity).

Sub Foo (TheArg)
If ItsWrong (TheArg)
Exit Sub 'Nuff said.
End If
'From here onwards TheArg is good stuff...
: : :
'Plenty more but not too much.
: : :
End Sub

When you rigidly stick to the 'Use A Boolean and Keep Checking It' rule to
control program flow, you can sometimes get tied in knots as bad as GoTos used
to be - trying to fight your way out of the code to reach the bottom. In these
cases a simple Exit Sub will cut the crap quite nicely. ;-)

One example, is search loops. The value gets found in the middle of the
loop. All I want to do is return it. There's a wonderful keyword - Return -
created just for the job and I'm happy to use it. If we weren't supposed to
exit before the end, languages would never provide Returns and Exit Sub.

If there's concern that a subsequent programmer will overlook the abrupt
exit - document it in lovely big capitals at the top of your routine (or not).
Seriously, though, it's easy enough to list the terminating conditions of a
routine.

Are you being 'being picky'? Well, let's change the phrase to 'being
choosy'. And now, because you're asking the question, we change it to 'giving
the matter consideration'. All of a sudden it sounds like a good idea to me.
Something that indicates the development of maturity and your own style. )

Regards,
Fergus

Thanks for that. I use Return a lot without considering that it is a
multiple exit point! But your exhortation to subscribe to rules only
insofar as they make sense, makes sense. I do tend to be a bit
obsessive with clean, conforming code.

Edward

 

[Cor]

Hi Edward,

Brackets help to remove ambiguity, and IMO aid readability and make
intended logic more clear.

I thought that always as well, but I am becoming in doubt if the Vb way is
not more easy to read.

But I think that I still more like the uniform way of the brackets.

Cor

 

[Tom Shelton]

Hi Edward,
Why not just

If not dataTable Is Nothing Then
if not colStyle Is Nothing Then
' do your things
end if
end if

It is VB not a C# like language with brackets on the most strange places
)
Cor

If Not dataTable Is Nothing AndAlso Not colStyle Is Nothing Then
' do your stuff
End If

 

[Jay B. Harlow [MVP - Outlook]]

Edward,
I would use the first as they are effectively the Guard Clause Pattern.

http://www.refactoring.com/catalog/replaceNestedConditionalWithGuardClauses.html

I may condense it further to:

If dataTable Is Nothing Then Exit Sub
If colStyle Is Nothing Then Exit Sub

Alternatively I would throw exceptions.

If (dataTable Is Nothing) Then

Throw New ArgumentNullException("dataTable")

End If
If (colStyle Is Nothing) Then

Throw New ArgumentNullException("colStyle ")

End If

Your second seems too easy to introduce spaghetti code.

Am I just being picky? I was taught that one should have a single
exit point in every function.

I do not feel the above violates the "single exit point rule", the single
exit point rule is intended to avoid duplicating clean up code just before
the exit of the routine.

The following I would consider to be in violation of the single exit point
rule.

Dim inStream As New FileStream(...)
Dim outStream As New FileStream(...)

If (dataTable Is Nothing) Then ...
inStream.Close()
outStream.Close()
Exit Sub
End If
If (colStyle Is Nothing) Then ...
inStream.Close()
outStream.Close()
Exit Sub
End If

...
inStream.Close()
outStream.Close()

Notice that I have duplicate close statements scattered across my function.
Here you should have a single exit point that closes the files.

Instead of setting a flag, I would consider a Finally clause of a Try Catch
block to allow a single exit point, something like:

Dim inStream As New FileStream(...)
Dim outStream As New FileStream(...)
Try

If (dataTable Is Nothing) Then ...
Exit Sub
End If
If (colStyle Is Nothing) Then ...
Exit Sub
End If

...
Finally
inStream.Close()
outStream.Close()
End Try

Of course I should have the file opens in the same Try Catch or an outer Try
Catch also. ;-)

Also per the above reference, I normally do not nest the conditional using
not logic. For a complete discussion of the "Replace Nested Conditional With
Guard Classes" you need to purchase Martin Fowler's Refactoring book. I
believe Guard Clauses are "defined" by Kent Beck in Smalltalk Best Practice
Patterns, however I do not have that book yet and I'm not finding a good web
reference.

Hope this helps
Jay

 

[Edward]

Edward,
I would use the first as they are effectively the Guard Clause Pattern.

What is the Guard Clause Pattern? You make it sound like Holy Writ!
Seriously, I bought the Gang of Four book but absolutely hated it.
And the code was pretty sucky too.

http://www.refactoring.com/catalog/replaceNestedConditionalWithGuardClauses.html

I sort of see what they are getting at, but I think there is a danger
of the resurgence of what I used to think of as "macho" coding in C,
where the most fantastically arcane constructions were used just to
save a couple of lines of code ...

I may condense it further to:

If dataTable Is Nothing Then Exit Sub
If colStyle Is Nothing Then Exit Sub

I personally don't care for If..Then clauses on one line - they are
gotchas waiting to happen if you're trying to read it (though of
course it executes just fine). If the typing is a problem, learn to
touch type. Makes it SO much more readable.

Alternatively I would throw exceptions.

Throw New ArgumentNullException("dataTable")
Throw New ArgumentNullException("colStyle ")

Yes, I like the sound of this.

Your second seems too easy to introduce spaghetti code.

Possibly, though I would probably rewrite it if there were more than a
couple more conditions.

I do not feel the above violates the "single exit point rule", the single
exit point rule is intended to avoid duplicating clean up code just before
the exit of the routine.

That's not what I was taught.

The following I would consider to be in violation of the single exit point
rule.

Dim inStream As New FileStream(...)
Dim outStream As New FileStream(...)

...
inStream.Close()
outStream.Close()

Notice that I have duplicate close statements scattered across my function.
Here you should have a single exit point that closes the files.

I wouldn't disagree with you here.

Instead of setting a flag, I would consider a Finally clause of a Try Catch
block to allow a single exit point, something like:

Dim inStream As New FileStream(...)
Dim outStream As New FileStream(...)
Try
...
Finally
inStream.Close()
outStream.Close()
End Try

Of course I should have the file opens in the same Try Catch or an outer Try
Catch also. ;-)

Also per the above reference, I normally do not nest the conditional using
not logic.

Huh? Sorry, I don't understand what you mean.

For a complete discussion of the "Replace Nested Conditional With
Guard Classes" you need to purchase Martin Fowler's Refactoring book. I
believe Guard Clauses are "defined" by Kent Beck in Smalltalk Best Practice
Patterns, however I do not have that book yet and I'm not finding a good web
reference.

Hope this helps
Jay

It's very kind of you, Jay, and all the others of course who have
given of their time here. There are many ways to flay a feline, and I
don't know that my way is particularly worse or better than any of the
alternatives. One can argue about this until the return of the kine,
and it's all just going to come down to personal preference.

Thanks again

Edward

 

[Jay B. Harlow [MVP - Outlook]]

Edward,

What is the Guard Clause Pattern?

As I said at the end of my post, I believe you need Kent Beck's book
"Smalltalk Best Practice Patterns" for initial definition of the Gaurd
Clause Pattern. Like I stated I however do not have that Kent Beck book, nor
can I find a good web reference.

You make it sound like Holy Writ!

Really, I'm sorry, didn't mean to. I was just providing 'back up' for my
answer. If you know or care about who Kent Beck & Martin Fowler are then my
backup should be validated, if you don't know or care who they are then I
guess my backup would not be validated. Other then they have well respected,
published books, I don't hold the individuals in any higher or lower regard
then you or I.

Seriously, I bought the Gang of Four book but absolutely hated it.
And the code was pretty sucky too.

Is it the book itself you object (no pun intended) to or the idea of using
Design Patterns in general?

If its the book itself, but you see value in Design Patterns. James W.
Cooper's "Visual Basic Design Patterns - VB 6.0 and VB.NET" from Addison
Wesley is a good companion book to the GOF book, that for the most part does
a good job of explaining each of the GOF patterns to VB6 & VB.NET
developers.

If you have some aversion to Design Patterns themselves, you may want to
read Cooper's book any way, as he may provide some insight as to why Design
Patterns are used and endorse by so many OOP programmers. It may trigger the
proverbial "Aha!" as to why so many others see value in Design Patterns. You
comment about 'Holy Writ' makes me believe you feel you do not have time for
Design Patterns.

Of course if you feel you have no time for Design Patterns in OOP theory I'm
O.K. with that also.

Yes, I like the sound of this.

Just out of curiosity, the Throw is effectively an exit, as you are
immediately leaving the current routine, why do you like the sound of Throw
over the sounds of Exit Sub? (this is not meant as a trick question!)

Huh? Sorry, I don't understand what you mean.

I would favor this:

If (dataTable Is Nothing) Then
Exit Sub
End If
If (colStyle Is Nothing) Then
Exit Sub
End If

Over this:

If not dataTable Is Nothing Then
if not colStyle Is Nothing Then
' do your things
end if
end if

This should not be surprising as I see value in Guard Clauses!

Hope this helps
Jay

"Jay B. Harlow [MVP - Outlook]" <[email protected]> wrote in

Edward,
I would use the first as they are effectively the Guard Clause Pattern.

What is the Guard Clause Pattern? You make it sound like Holy Writ!
Seriously, I bought the Gang of Four book but absolutely hated it.
And the code was pretty sucky too. http://www.refactoring.com/catalog/replaceNestedConditionalWithGuardClauses.html

I sort of see what they are getting at, but I think there is a danger
of the resurgence of what I used to think of as "macho" coding in C,
where the most fantastically arcane constructions were used just to
save a couple of lines of code ...

I may condense it further to:

If dataTable Is Nothing Then Exit Sub
If colStyle Is Nothing Then Exit Sub

I personally don't care for If..Then clauses on one line - they are
gotchas waiting to happen if you're trying to read it (though of
course it executes just fine). If the typing is a problem, learn to
touch type. Makes it SO much more readable.

Alternatively I would throw exceptions.

Throw New ArgumentNullException("dataTable")
Throw New ArgumentNullException("colStyle ")

Yes, I like the sound of this.

Your second seems too easy to introduce spaghetti code.

Possibly, though I would probably rewrite it if there were more than a
couple more conditions.

I do not feel the above violates the "single exit point rule", the single
exit point rule is intended to avoid duplicating clean up code just before
the exit of the routine.

That's not what I was taught.

The following I would consider to be in violation of the single exit point
rule.

Dim inStream As New FileStream(...)
Dim outStream As New FileStream(...)

...
inStream.Close()
outStream.Close()

Notice that I have duplicate close statements scattered across my function.
Here you should have a single exit point that closes the files.

I wouldn't disagree with you here.

Instead of setting a flag, I would consider a Finally clause of a Try Catch
block to allow a single exit point, something like:

Dim inStream As New FileStream(...)
Dim outStream As New FileStream(...)
Try
...
Finally
inStream.Close()
outStream.Close()
End Try

Of course I should have the file opens in the same Try Catch or an outer Try
Catch also. ;-)

Also per the above reference, I normally do not nest the conditional using
not logic.

Huh? Sorry, I don't understand what you mean.

For a complete discussion of the "Replace Nested Conditional With
Guard Classes" you need to purchase Martin Fowler's Refactoring book. I
believe Guard Clauses are "defined" by Kent Beck in Smalltalk Best Practice
Patterns, however I do not have that book yet and I'm not finding a good web
reference.

Hope this helps
Jay

It's very kind of you, Jay, and all the others of course who have
given of their time here. There are many ways to flay a feline, and I
don't know that my way is particularly worse or better than any of the
alternatives. One can argue about this until the return of the kine,
and it's all just going to come down to personal preference.

Thanks again

Edward

 

[Edward]

Edward,
As I said at the end of my post, I believe you need Kent Beck's book
"Smalltalk Best Practice Patterns" for initial definition of the Gaurd
Clause Pattern. Like I stated I however do not have that Kent Beck book, nor
can I find a good web reference.

Thanks for the refresher. I will check this out and see if my
employers will cough for it.

Is it the book itself you object (no pun intended) to or the idea of using
Design Patterns in general?

Well, I thought the style was ineffably smug. However, my meagre
intellect was utterly unable to project the academic ideas into the
real world. What use were they to me, these patterns? I could see no
congruence with my work as a developer.

If its the book itself, but you see value in Design Patterns. James W.
Cooper's "Visual Basic Design Patterns - VB 6.0 and VB.NET" from Addison
Wesley is a good companion book to the GOF book, that for the most part does
a good job of explaining each of the GOF patterns to VB6 & VB.NET
developers.

I have heard of this book, and will check it out too.

If you have some aversion to Design Patterns themselves, you may want to
read Cooper's book any way, as he may provide some insight as to why Design
Patterns are used and endorse by so many OOP programmers. It may trigger the
proverbial "Aha!" as to why so many others see value in Design Patterns. You
comment about 'Holy Writ' makes me believe you feel you do not have time for
Design Patterns.
Just out of curiosity, the Throw is effectively an exit, as you are
immediately leaving the current routine, why do you like the sound of Throw
over the sounds of Exit Sub? (this is not meant as a trick question!)

How funny. I was only a few minutes ago on my way into work thinking
just this very thing. It's like the old joke about people trying to
lose weight - calories don't count if you get them off someone else's
plate! My aversion to Exit Sub is that I took over a number of
projects from someone who used it indiscriminately, and I personally
found that it made code both hard to read and hard to debug.

I would favor this:

Over this:

I'm 100% with you here. I am constantly finding people checking for
NOT something, when checking for something is just as easy.

This should not be surprising as I see value in Guard Clauses!

Well, give me a few weeks and I may know what you're talking about!

Hope this helps

Certainly does. Many thanks again.

Edward

 

[EricJ]

only about the not statement

I'm 100% with you here. I am constantly finding people checking for
NOT something, when checking for something is just as easy.

by using not you can avoid having a if then else structure with an empty if
condition
if boolean then
else
'code
end if

if not boolean then
'code
end if

or to make it more readable ex:
if not blnValidated then
'make the user correct his mistake
end if

last use of not i'm going to bore you with ;p yes there are more

blnTest = not blnTest
'this can come in handy 2, in a sub that sets the visible state of a lot of
components for example (1sub for both visible and invisible)

 

[Cor]

Hi Eric,
It was funny I saw this, this is something I had sended in and Jay B took it
in this thread as not good coding without direct answering me. And although
I have a lot of respect for Jay B, I can tell you I had a not so good
feeling.

I was making an answer and then I thought lets first see what Eric wrote.
And it was alomost the same as I did want to write.

When I made this example I had a datatable in mind that probably could have
no rows and did not even look to it that it was the colStyle. But because
the same as what you are writting I find this the nicest and just use it
without thinking.

The use of an exist sub is one of the things, that has for me the same as a
go to, and about that part of the discussion I do not start again, to much
in history

(You can use it, but if you can avoid it you should do that I think)

For this is I think the sample Tom Shelton did give was the best
\\\
If Not dataTable Is Nothing AndAlso Not colStyle Is Nothing Then
' do your stuff
End If
///

Funny.

Cor

 

[Cor]

Hi Jay B,

When you have critique on my code, please do it in the original thread?

I have a lot of respect for you, and this does not change that, but this I
don't like.

Cor

 

[Jay B. Harlow [MVP - Outlook]]

Edward,
I found a like to the Guard Clause Pattern.

http://c2.com/cgi/wiki?GuardClause

Hope this helps
Jay

"Jay B. Harlow [MVP - Outlook]" <[email protected]> wrote in

Edward,
As I said at the end of my post, I believe you need Kent Beck's book
"Smalltalk Best Practice Patterns" for initial definition of the Gaurd
Clause Pattern. Like I stated I however do not have that Kent Beck book, nor
can I find a good web reference.

Thanks for the refresher. I will check this out and see if my
employers will cough for it.

Is it the book itself you object (no pun intended) to or the idea of using
Design Patterns in general?

Well, I thought the style was ineffably smug. However, my meagre
intellect was utterly unable to project the academic ideas into the
real world. What use were they to me, these patterns? I could see no
congruence with my work as a developer.

If its the book itself, but you see value in Design Patterns. James W.
Cooper's "Visual Basic Design Patterns - VB 6.0 and VB.NET" from Addison
Wesley is a good companion book to the GOF book, that for the most part does
a good job of explaining each of the GOF patterns to VB6 & VB.NET
developers.

I have heard of this book, and will check it out too.

If you have some aversion to Design Patterns themselves, you may want to
read Cooper's book any way, as he may provide some insight as to why Design
Patterns are used and endorse by so many OOP programmers. It may trigger the
proverbial "Aha!" as to why so many others see value in Design Patterns. You
comment about 'Holy Writ' makes me believe you feel you do not have time for
Design Patterns.
Throw New ArgumentNullException("dataTable")
Throw New ArgumentNullException("colStyle ")
Just out of curiosity, the Throw is effectively an exit, as you are
immediately leaving the current routine, why do you like the sound of Throw
over the sounds of Exit Sub? (this is not meant as a trick question!)

How funny. I was only a few minutes ago on my way into work thinking
just this very thing. It's like the old joke about people trying to
lose weight - calories don't count if you get them off someone else's
plate! My aversion to Exit Sub is that I took over a number of
projects from someone who used it indiscriminately, and I personally
found that it made code both hard to read and hard to debug.

I would favor this:

Over this:

I'm 100% with you here. I am constantly finding people checking for
NOT something, when checking for something is just as easy.

This should not be surprising as I see value in Guard Clauses!

Well, give me a few weeks and I may know what you're talking about!

Hope this helps

Certainly does. Many thanks again.

Edward

 

[Fergus Cooney]

Hi Cor,

I hate
If Not oSomething Is Nothing Then

It's like the VB equivalent of
"I didn't say nothing"

except that it actually means the double negative.

Regards,
Fergus

[Not yet gone, but still pondering. Thanks for your words. Perhaps I should
put it to the group. Any thoughts? ;-)]

 

[Cor]

Hi Fergus,

Good to see you

I hate
If Not oSomething Is Nothing Then
It's like the VB equivalent of
"I didn't say nothing"

I would full agree if there was a
I did say it

And that therefore I accept it

If oSomething Is True Then
Would be a lot better.

And I think do not put it in the group, that makes from you a Victim and you
are not.

Cor