Exit Stage Left
Larry Hengen - 08/May/2020
Larry Hengen - 08/May/2020
[SHOWTOGROUPS=4,20]
The Exit procedure has been around as long as Goto in Object Pascal.
Goto is considered an antiquated command, but what about about Exit?
When should it be used if at all?
I have often seen guard code at the beginning of a method that performs an Exit if the parameters passed are not appropriate or indicate the method does not need to act on the parameters passed. This might happen in a method like this:
In this case the first few lines are a common place to see such guard code. In fact, guard code is usually only in place to throw exceptions to let the developer know prerequisites have not been met. Exit calls may be more applicable depending on the purpose of the method but I would caution that taking this to extreme is the equivalent of making a function that returns a boolean for success and then never checking the result in the calling code. It’s a bad practice that will one day bite you.
Pick a strategy, document it, and adhere to it. Consistency is king even if the code is less than ideal. Consistent code is easier to re-factor later.
Anyway, I digress, contrast the first method to the following:
Here a private field variable is accessed within the called method to determine if the called method should in fact have been called. Seems a little “ass backwards” to me. You don’t go to Vegas and then ask yourself “should I be in Vegas?”. You think before you act. If the method should not have been called, then the caller should make that determination and simply not call it. There is overhead with every method call.
What has also happened now, is that the CheckHaveID method is coupled to the form, making refactoring harder. This is the problem with business logic being tied to a form. Business logic needs to be applied throughout the application but it can’t do that if some variation is embedded in all related forms. Let’s say I want to pull the CheckHaveID method out into another unit as a classless or static class method so it can be shared in another form. Now I have to deal with changing the calling logic to account for the FIShouldCallCheckHaveID condition. If this had been done in the first place the risk of breaking code would have been non-existent.
The argument can be made that if you have 5 early Exits these tests should not be duplicated in every place the method is called, that it just doesn’t make sense. I would respond to such a statement that code duplication is a primary code smell that should result in re-factoring, In this case, all 5 instances of the calling code would be extracted into another method where the decision would be made. Just because putting the logic test in guard code eliminates duplication doesn’t mean you’re putting it in the right place.
In my opinion conditional flow logic should be implemented in the caller. In keeping with the Single Responsibility Principle, a method should do one and only one thing and it’s name should reflect exactly what it does. This keeps code readable and understandable. That was the whole purpose behind step-wise refinement I was taught in Computer Science. Embedding business logic (execution flow) in a method that is named DoX means it should not also do Y. Such code requires minimal documentation because it’s name says it all (well almost).
Structured programming purists would say never use Exits, but more modern engineers like Martin Fowler are advocate smaller methods with multiple Exits. The keyword here is smaller methods. In legacy code I’ve seen methods spanning several hundred lines. Try to find an Exit call in that code!
Not using Exit calls can certainly increase the level of nested if..then..else statements which can also be hard to follow and lead to bugs if begin/end blocks are not used. Again size does matter (method size).
I think as with most things in life, there is a balance, and at a certain point it becomes a personal choice but I think in at least this case, I have made an argument for avoiding the code shown.
Warning : use with caution - jumping is a concept at odds with structured coding - it makes code maintenance difficult.
[/SHOWTOGROUPS]
The Exit procedure has been around as long as Goto in Object Pascal.
Goto is considered an antiquated command, but what about about Exit?
When should it be used if at all?
I have often seen guard code at the beginning of a method that performs an Exit if the parameters passed are not appropriate or indicate the method does not need to act on the parameters passed. This might happen in a method like this:
Код:
procedure TSomeForm.CheckHaveID(const AID: string);
begin
if AID <> EmptyStr then
Exit;
...other code to populate AID if necessary...
end;In this case the first few lines are a common place to see such guard code. In fact, guard code is usually only in place to throw exceptions to let the developer know prerequisites have not been met. Exit calls may be more applicable depending on the purpose of the method but I would caution that taking this to extreme is the equivalent of making a function that returns a boolean for success and then never checking the result in the calling code. It’s a bad practice that will one day bite you.
Pick a strategy, document it, and adhere to it. Consistency is king even if the code is less than ideal. Consistent code is easier to re-factor later.
Anyway, I digress, contrast the first method to the following:
Код:
procedure TSomeForm.CheckHaveID(const AID: string);
begin
if not FIShouldCallCheckHaveID then
Exit;
if AID <> EmptyStr then
Exit;
...other code to populate AID if necessary...
end;Here a private field variable is accessed within the called method to determine if the called method should in fact have been called. Seems a little “ass backwards” to me. You don’t go to Vegas and then ask yourself “should I be in Vegas?”. You think before you act. If the method should not have been called, then the caller should make that determination and simply not call it. There is overhead with every method call.
What has also happened now, is that the CheckHaveID method is coupled to the form, making refactoring harder. This is the problem with business logic being tied to a form. Business logic needs to be applied throughout the application but it can’t do that if some variation is embedded in all related forms. Let’s say I want to pull the CheckHaveID method out into another unit as a classless or static class method so it can be shared in another form. Now I have to deal with changing the calling logic to account for the FIShouldCallCheckHaveID condition. If this had been done in the first place the risk of breaking code would have been non-existent.
The argument can be made that if you have 5 early Exits these tests should not be duplicated in every place the method is called, that it just doesn’t make sense. I would respond to such a statement that code duplication is a primary code smell that should result in re-factoring, In this case, all 5 instances of the calling code would be extracted into another method where the decision would be made. Just because putting the logic test in guard code eliminates duplication doesn’t mean you’re putting it in the right place.
In my opinion conditional flow logic should be implemented in the caller. In keeping with the Single Responsibility Principle, a method should do one and only one thing and it’s name should reflect exactly what it does. This keeps code readable and understandable. That was the whole purpose behind step-wise refinement I was taught in Computer Science. Embedding business logic (execution flow) in a method that is named DoX means it should not also do Y. Such code requires minimal documentation because it’s name says it all (well almost).
Structured programming purists would say never use Exits, but more modern engineers like Martin Fowler are advocate smaller methods with multiple Exits. The keyword here is smaller methods. In legacy code I’ve seen methods spanning several hundred lines. Try to find an Exit call in that code!
Not using Exit calls can certainly increase the level of nested if..then..else statements which can also be hard to follow and lead to bugs if begin/end blocks are not used. Again size does matter (method size).
I think as with most things in life, there is a balance, and at a certain point it becomes a personal choice but I think in at least this case, I have made an argument for avoiding the code shown.
Warning : use with caution - jumping is a concept at odds with structured coding - it makes code maintenance difficult.
[/SHOWTOGROUPS]