To clarify my feelings on the Single Exit Point issue, I'm for it in general. With one very Delphi-specific exception that I think falls into the "error handling" exception that has some support in the community.
To clarify my feelings on the Single Exit Point issue, I'm for it in general. With one very Delphi-specific exception that I think falls into the "error handling" exception that has some support in the community.
Here's where I will often use a mid-function exit. It's a nice short method and it's clear what is happening (IMO, anyway).
function PanelTypeFromString( const Str:String):TPanelType;
begin
for Result := low(TPanelType) to high(TPanelType) do
if PanelInfoFull[Result].Short = Str then
Exit;
raise RelevantException.Create(...);
end;
Refactoring it to avoid the exit means using a while and duplicate tests:
function PanelTypeFromString2( const Str:String):TPanelType;
begin
Result := low(TPanelType);
while (Result < pred(high(TPanelType))) and (PanelInfoFull[Result].Short <> Str) do
Inc(Result);
if (Result = high(TPanelType)) and (PanelInfoFull[Result].Short <> Str) then
raise RelevantException.Create(...);
end;
I think the first form is easier to understand. It's also shorter without being obfuscated, which I like. The second could use range checking and throw an out of range error but IMO that's no better than the above.
I wonder how much of my dislike of mid-exits is because I associate them with code that has really long methods and rather than splitting them they just jump out half way. Which may make it a refactoring hint.
Can we please try to have a non-dogmatic discussion of this.
Here's where I will often use a mid-function exit. It's a nice short method and it's clear what is happening (IMO, anyway).
function PanelTypeFromString( const Str:String):TPanelType;
begin
for Result := low(TPanelType) to high(TPanelType) do
if PanelInfoFull[Result].Short = Str then
Exit;
raise RelevantException.Create(...);
end;
Refactoring it to avoid the exit means using a while and duplicate tests:
function PanelTypeFromString2( const Str:String):TPanelType;
begin
Result := low(TPanelType);
while (Result < pred(high(TPanelType))) and (PanelInfoFull[Result].Short <> Str) do
Inc(Result);
if (Result = high(TPanelType)) and (PanelInfoFull[Result].Short <> Str) then
raise RelevantException.Create(...);
end;
I think the first form is easier to understand. It's also shorter without being obfuscated, which I like. The second could use range checking and throw an out of range error but IMO that's no better than the above.
I wonder how much of my dislike of mid-exits is because I associate them with code that has really long methods and rather than splitting them they just jump out half way. Which may make it a refactoring hint.
Can we please try to have a non-dogmatic discussion of this.
While I find the mid exit perfectly fine in your example, many times, especially when there is more then one mid exit and the procedure body is longer I find it hard to understand the code flow (Even for code I had written myself and looked at it after a few month). So all in all I think we should take into account the common sense that "writing code is easier than reading code" and act or counteract accordingly. So for your example I typically use something along this construct:
ReplyDeletefunction PanelTypeFromString( const Str:String):TPanelType;
var
PanelTypeFound: Boolean;
begin
PanelTypeFound := False;
for Result := low(TPanelType) to high(TPanelType) do
if PanelInfoFull[Result].Short = Str then
begin
PanelTypeFound := True;
Break;
end;
if not PanelTypeFound then
raise RelevantException.Create(...);
end;
Using such a construct the code flow is from begin to end of the procedure body. Again for such short functions as the one in your example I think the mid exit is perfectly fine, while for more complex code I would try to avoid something like:
fucntion xy(param xyz: whaver): AnyType
begin
if condition1 and condition2 then
Exit(something) else
if (conditon3 and param) or not param then Exit(soemthing else) else
Result := AgainSoemethingElse
end;
I prefer early exits, especially if the alternative is nesting. Mid-exits can be a bit nasty unless the method fits on one screen.
ReplyDeleteMid-exits can usually be refactored to a flat "early exit" style by putting stuff into functions though.
I'm all about guard clauses. They make life sane. In loops with complex conditional clauses, I'm not against putting guard clauses to continue or break the loop either, though I'll try to refactor when I can.
ReplyDelete