Why do people use Exit Sub and Goto?

[foxguy]

I notice that a lot of people use this format for macros.

Sub Worksheet_Change(Target as Range)
    On Error GoTo HandleError
    
    If Target.Count <> 1 Then Exit Sub
    Application.EnableEvents = False
    '  my Code
CleanUp:
    Application.EnableEvents = True
    Exit Sub
HandleError:
    If Error_Handled() Then Resume
    Goto CleanUp
End Sub

1) I was taught that it is good form to have only 1 exit from a sub, and it should be at the end of the sub, not somewhere in the middle. This code has 2 "Exit Sub"s from the middle, and it cannot exit at the end.

2) I was also taught that the flow should always be in 1 direction (except for loops), it shouldn't have any "GoTo"s that go backwards. This code has the "GoTo CleanUp" that goes backwards.


My macros use this format, that avoids both issues.

Sub Worksheet_Change(Target as Range)
    On Error GoTo HandleError
    Application.EnableEvents = False
    If Target.Count = 1 Then
        '  my Code
    End If
HandleError:
    If Err.Number <> 0 Then
        If Error_Handled() Then Resume
    End If
CleanUp:
    Application.EnableEvents = True
End Sub

Did the rules change after I learned? Does anyone have any comments about why mine is not good? Are there any web sites where this issue is discussed (especially sites that argue for the 1st method)?

[Simon Lloyd]

Your right that Goto should be avoided as it leads to spaghetti coding and you can soon get lost, the

If Target.Count <> 1 Then Exit Sub

used there is a valid Exit just as you pointed out they could have used = 1, this particular abrupt exit is before the main body of code has been executed so i see no real problem with that. Your code now no longer needs CleanUp: as it does nothing, i also see no reason why the first code would need a goto just to enable events which, in this instance, would serve no purpose in being located anywhere else other than the end of the code.

[foxguy]

Hi Simon;
Yes, this example is rather over simplified. I'm more interested in the concept.

Your code now no longer needs CleanUp: as it does nothing

In this particular example, nothing is done, but in the real world, I may not be able to easily see that I don't need to CleanUp. The Exit Sub might not be the very first thing in the macro, or the EnableEvents might have been done in a called sub from somewhere in the middle of the macro, so I wouldn't dare trust that CleanUp wasn't needed. I certainly don't want to spend my time trying to track down what might have happened. I would just automatically Cleanup. And that means going backwards in this example.

[DonkeyOte]

FWIW, I think the simplified example does raise the interesting point re: people's usage of:

If Target.Count <> 1 Then Exit Sub

in a Change event ... which whilst I concede is not totally relevant to the main question here (though does raise issue re use of Exit Sub) it is [IMO] worth highlighting nonetheless.

All too often people use this code and I think it's very poor design

Either

a) multi cell updates are permitted

b) multi cell updates are not permitted

If multi cell updates are not permitted then the action should be undone as opposed to ignored.
Using the above code in a Change Event I can circumvent the routine altogether simply by updating more than one cell simultaneously.
(which could for ex. lead to invalid entries persisting)

If multi cell updates are permitted (as is normally the case) then one should be looking to iterate the intersect of the Target range and the relevant range for the Change event.
(Repeating subsequent actions as necessary or where possible processing en masse).

Apologies for sidetrack but this one those things that we see all too often and I confess I find it strange how something so fundamental is so often overlooked.
I would say if you pay attention to the Change Event code that gets posted you'd be surprised just how prevalent this is.

[foxguy]

Hi DonkeyOte;

I thought I was the only one who noticed that. I admit that I have been lazy when I'm posting code here on the forum and do the same thing myself, just because it confuses too many beginners to do it correctly.

[mikerickson]

"One way in, one way out" is good practice, but error code is generally recognized as an exception.

This code violates "one way in, one way out", but could one think of it as a user generated error?

If MsgBox("Continue?", vbYesNo) = vbNo Then Exit Sub

One other reason that one sees code like that on the forum is that that lay out hightlights the point of the post.

If Target.Cells.Count <> 1 Or Target.Column <> 1 Then Exit Sub

' code to handle one cell

highlights the code to handle one cell. Burying it in machinery isn't as educational. (IMO)

Dim oneCell as Range

For each oneCell in Applicaiton.Intersect(Columns(1),Target)
    'code to handle one cell
Next oneCell

[Simon Lloyd]

I guess i have to hold my hands up as i code to the op's understanding as much as i can, i try not to confuse the op or deny them the possibility of understanding how the code works or flows, they'll learn there are many better ways as they progress, but if we code in a fashion that they can hardly understand or relate to they will forever be in the same position and never progress.

I may be way off here or even get stoned to death for the above but i remember when i started out and need it SIMPLE, so does that excuse spaghetti code? no definitely not as that is confusion in itself, does it mean i should EXIT SUB rather than undo then in some cases yes as the op can relate to that, does it mean i should goto NEVER!!!!!

[foxguy]

DonkeyOte;

Have you found a better way to be able to "UnWind" a change other than to store the cell's value in the SelectionChange event?

[Mordred]

Being relatively new to VBA coding, my question to you pros is: Does it really matter? If one snippet of code does the same thing as a different snippet of code (which happens in other programming languages), then doesn't it come down to personal design preference? I realize that, as I get better at VBA coding, my style will change to accommodate the project I am working on, it already has. But, in saying that, no one's style will be the same as another's style. So who's style is right?
Personally, I like using an If statement to handle possible errors, and then Exit Sub if need be. As far as Goto, I haven't had much experience with it yet so I just don't know.
Anyhow, if someone could gently tell me if it is more about writing style, or is it about 'proper' coding techniques?

[mikerickson]

Mordred,

If you have two codes that achieve the same end, then you can ask yourself "In six months, when I have to update this, will I be able to figure out what's what?" "Six months from then, will some stranger be able to modify the code to their needs."

"One way in, one way out" makes modifying code easier. A simple modification like "and then make the cell red" should go after everything else is done.
Following the principle of "one way in, one way out" puts "after everything else is done" in one place.

[Mordred]

mikerickson,
I agree with the one way in, one way out concept. Personally, I've been trying to write code that is generic to the point where I can copy and paste methods and use them in a different workbook. For errors though, isn't that where the exception has to be? I think I have 3 methods, so far, that will exit a sub if something does not comply.

[mikerickson]

Inside a project, error handling can be done a variety of ways. The biggest issue is when one routine calls another and the error is in the second. Exit Sub might pass the error back to the first routine. End is another option. Setting some kind of flag so the error can be checked by the first routine is another option.

In the case of error handling "Garbage in, garbage out" is a goal (as opposed to "Garbage in, dump truck breaks")

(Which is why I'm always specifyting CStr(aCell.Value), just in case there's an error value (eg DIV/0) in a cell.)

[shg]

Which is why I'm always specifyting CStr(aCell.Value),

How about just aCell.Text?

[mikerickson]

Using aCell.Text
1) returns "###" if the column is too narrow.
2) limits one's use of number formatting, especially with dates and times. DateValue and TimeValue can be used when reading aCell.Text, but CStr() is my choice for a quick, protect against the unlikely error value situation.