Is considered a bad practice or exist any drawback using try/finally try/except instead of begin/end?

Question

In many places in some Apps which I maintain , I've found code which uses a try/finally or try/except block in a for loop or if sentence avoiding the use of begin/end

Consider the next code (not production code, just a sample)

{$APPTYPE CONSOLE}

{$R *.res}

uses
  Classes,
  SysUtils;
    
Procedure TestNoBeginEnd;
var
 i   : Integer;
 L1  : TStringList;
begin
    for i := 1 to 10 do
    try
      L1:=TStringList.Create;
      try
        L1.Add('Bar');
        L1.Add(IntToStr(i));
        L1.Add('Foo');
      finally
        Writeln(L1.Text);
        L1.Free;
      end;
    except
      on E: Exception do
        Writeln('Opps '+E.ClassName, ': ', E.Message);
    end;
end;

begin
  try
    TestNoBeginEnd;

  except
    on E: Exception do
      Writeln(E.ClassName, ': ', E.Message);
  end;
  Readln;
end.

The Question, Is considered a bad practice, code smell or exist any drawback using a try/finally or try/except instead of begin/end in delphi?

UPDATE

I'm sorry by the silly sample code, just for clarify the try/finally and try/except doesn't pretend replace the begin/end , just to avoid to use it (the begin/end) when exist a case when the use of the try/finally or the try/except doesn't requires a begin/ end.

Answer 1

I personally don't see the need to add the extra begin..end around a try..finally/except..end, especially when there is little chance of code being added just before the try or certainly after the end.

That being said, the question would be more whether we need systematically a begin..end after a ..do or a ..then even when there is only a single instruction (in your case a try.. instruction).

It's mostly a question of style and habit, some people prefer to code defensively and systematically put a begin..end, others try to avoid unnecessary lines as much as possible.

When a fair number of lines are involved, I tend to add the begin..end to make the scoping more obvious.

I'd say, use good judgment and see what the chances are that someone (or you) modifying the code later could miss the intended scope.

Answer 2

First things first. Begin/End is not the same as Try/Etc. The latter first denotes a set of instructions, while the second is a single instruction (which contains possibly lots of instructions).

So they are like a hammer and ladder. Different tools for different jobs.Sure you drive a nail with a ladder but it's not the most efficient way of dealing with the problem.

Edit: If I undertand the question (now that you've edit it) you're asking if one try/etc can be used with out a begin/end. Yes you can do this, because try/etc is a single statement. In fact I'd go as far as to say it's probably a good thing to avoid too much no use code.

Before edit:

So what does that mean? Well it means that if an error occurs in that function it's gets logged, but essentially ignored.

Unless there is really good exception handling and the code escalates exceptions or recovers correctly I'd say this was bad practise.

In your sample all the exception handler does is say Opps (oops?). What good is that? Sure it's probably only a sample and as such I'd examine the code to see what it really does. I beleive that the person who wrote this was trying to handle errors generally, but is actually making the reliability of the code far far worse using this pattern.

I use the general case of 'Exception Handlers are for Exceptional Cirmcumstances' where I can recover or I need to let someone/thing know. Mostly I find that no one can handle the error and the system has to degrade as per design. This is where the exception handlers will log safely and pass teh exceptions on.

The latter case I extend to all 'big boundaries in the system'. i.e. I'll collect exceptions at Service/API boundaries and log/wrap appropriately.

(note sometimes bad APIs use Exceptions for non exceptional circumstances, in this case you can safely add an exception handler to wrap the bad design).

Answer 3

Your edit on the question radically changed the question. You should have changed the title at least, and right now I wonder if the question is salvageable at all (-1).

That said, when addressing your original question:

A Try/Finally or Try/Except statement should never be used when a Begin/End statement would suffice. Besides the fact that Try-statements add (tiny or even unnoticeable) extra instructions to the compiled code, they imply code necessities which aren't there:

for I := 0 to Count - 1 do
try
  Inc(Rect.Left, Shift);
finally
  Inc(Rect.Right, Shift);
end;

Just not do it.

Answer 4

There is nothing wrong with omitting the begin end when a try finally/except wraps the same code in a block for you. Just remember that try-blocks have more overhead than a begin-block. This overhead is usually negligible, but I have found it to make meaningful differences when in loops, specially loops in methods that are referenced repeatedly.

Taking the above details in mind, your example is somewhat poor due to the fact that it adds unneeded overhead repeatedly in the loop. Whenever you can move try blocks out of a loop, it's a good thing. In your example if you MUST have the ability to catch an exception for each loop iteration, the below code would be more efficient, if not, move the except block out also.

Procedure TestNoBeginEnd;
var
 i   : Integer;
 L1  : TStringList;
begin
  L1:=TStringList.Create;
  try
    for i := 1 to 10 do
    try
      L1.Clear;
      L1.Add('Bar');
      L1.Add(IntToStr(i));
      L1.Add('Foo');
      Writeln(L1.Text);
      end;
    except
      on E: Exception do
        Writeln('Opps '+E.ClassName, ': ', E.Message);
    end;
  finally
    L1.Free;
  end;
end;