Best practices concerning exit in Delphi

Question

A co-worker and myself are having a debate on whats best. Both concepts work and work well but is there a general consensus on not making a call to exit?

Whats better?

To call exit within a procedure to avoid doing the rest of the code as in ...

if Staging.Current = nil then exit  

DoSomethingA(FileNameA);  
DoSomethingB(FileNameB);  
Staging.DeleteCurrent;

or to not call exit and instead wrap it in a begin and end

if Staging.Current <> nil then  
begin   
  DoSomethingA(FileNameA);  
  DoSomethingB(FileNameB);  
  Staging.DeleteCurrent;   
end;

Both work. I prefer to use the exit statement since it results in fewer lines of code and looks cleaner to me, but is there any reason or any consensus among programmers to avoid using exit to leave a procedure?

Answer 1

This is a religious war type of question. For reference the definitive Stack Overflow discussion is here: https://stackoverflow.com/questions/36707/should-a-function-have-only-one-return-statement

Many people object to methods that have multiple exit points. They would argue that it makes it harder to reason about a method's behaviour.

On the other hand, others take the point of view that once a method has completed its work it is reasonable for it to quit. Those holding this viewpoint would argue that a C return statement, e.g. return 42 is clear and reasonable anywhere in a function.

I personally feel that there is a clear distinction between code that exits willy nilly from many different points, and code as presented in your question which is known as a guard clause.

One of the great advantages of guard clauses is when you have multiple tests. The code written without guard clauses results in significant indentation which most people agree is to be considered harmful.

It is my perception that the consensus opinion is that guard clauses are better than any alternative.

Answer 2

The only reason (besides your own preferences) for not using exit are programming guidelines. If no such guideline exists feel free to use whatever fits your needs or makes your code look cleaner.

Edit: More recent versions of Delphi allow a call to Exit with a parameter which becomes the result of the function (if it is a function at all). This might lead to some leaner and (subjective view) cleaner code in some cases.

Example:

function IndexOfString(const Value: string; const StringArray: array of string): Integer;
begin
  for I:=Low(StringArray) to High(StringArray) do 
    if Value = StringArray[I] then 
      Exit(I);
  result := -1;
end;

Another approach for this would be:

function IndexOfString(const Value: string; const StringArray: array of string): Integer;
begin
  result := -1;
  for I:=Low(StringArray) to High(StringArray) do 
    if Value = StringArray[I] then begin
      result := I;
      Break;
    end;
end;

Whatever is the better one is more or less a matter of taste.

Answer 3

An early exit like this is called a "guard clause". It is not unique to Delphi. Increasingly, it is considered to be good practice.

Advantages:

• Less lines of code.
• If is +ve, making it easier to understand.
• Less nesting = easier to understand.

Disadvantages:

• If your method allocates resources, it is easy to forget to release them if you don't have a single exit point. For this reason, this style of programming may be more suited to managed languages like C#, but can be used with Delphi as long as you're aware of the issue.

I'm not convinced by arguments that a method should have a single exit point. It has been suggested to me that an early return is just another kind of GOTO. However, the problem with GOTO is that it can take you anywhere, leading to the most horrible mess when it is misused. On the other hand, an early exit from a method always returns you to the caller. What could be neater than that?

Answer 4

Subjective question. I often use Exit in a sense to check for some initial conditions and exit early if they are not met. My functions often look like this:

begin
  Result := ''; // default return value
  if (<input does not meet conditions>) then
    Exit;

  // continue to do something and return something useful
end;

This makes it a bit more readable for me. But I try not to use Exit from various places in the same function.

Answer 5

Personally, I do no think there is any need to avoid the use of exit. It's useful in functions like:

function IsDivideable(const A, B: Integer): Boolean;
begin
  Result := False;

  if (B = 0) then
     exit;

  Result := ((A mod B) = 0);
end;

Answer 6

Sometimes Exit might "break" method readability. Kind of like goto would (only "less bad"). I myself prefer conditional blocks, which make very clear the scope of each chunk of code.

Answer 7

I try to not use Exits in code. If you like the code provided, then I'd prefer changing it to be more positive thinking as it typically flows better:

  if Assigned(Staging.Current) then  
  begin   
    DoSomethingA(FileNameA);  
    DoSomethingB(FileNameB);  
    Staging.DeleteCurrent; 
  end;

There's always more than one way of doing something - the deciding factor really is what implementation is the easiest for you and your team to maintain. This is typically defined by writing easier to read and understand code. Burying an exit in the middle of a procedure is generally a bad implementation, on the flip side having a simple one-liner guard code up front would likely fall into the easy to read/understand side for most.

Answer 8

I would agree with other posters that Exit in my view reduces readability. However if you are going to use it (and in certain circumstances it does improve readability) formatting is very important. I would change your above to

if Staging.Current = nil then exit

DoSomethingA(FileNameA);
DoSomethingB(FileNameB);
Staging.DeleteCurrent;

The extra line between if and the rest of the statements is important.

Answer 9

I'm using Exit when I have a complex condition in the method, and after a while the specs change and you must add some new conditions to code, and it would cost time to rearrange all the conditions. Depends from situation to situation.

But, if the logic is simple, the I prefer to check the stuff and finish the method if everything is ok.

LE: another discussion on this Code Style - Do you prefer to return from a function early or just use an IF statement?

Answer 10

I would not consider it as good or bad. Like all answers are showing, it might be hardly depending on the case.

Personally I like the simple "guard condition" with no return value, but I would want to use it in complex functional structure or with class return values.

Maybe some best practices could be extracted from the answers above, and could lead to a simple checklist, when its safe and useful to use "Exit", and also when to better avoid it.