In delphi 7, is `try ... except raise; end;` meaningful at all?

Question

In some Delphi 7 code I am maintaining, I've noticed a lot of instances of the following:

with ADOQuery1 do begin
  // .. fill out sql.text, etc
  try
    execSQL;
  except
    raise;
  end;
end;

It seems to me that these try blocks could be removed, since they do nothing. However, I am wary of possible subtle side-effects..

Can anyone think of any instances in which these blocks could actually do anything that wouldn't happen without them there?

Brade

See also: #717989 – Saltire

Answer 1

In this context, the raise operation has no effect and should be removed becuase its simply re-raising the exception that the exception block just caught. raise is typically used to transfer control to the end of the block when no appropriate error handling is available. In the following we handle the custom exception, but any other exception should be handled elsewhere.

try
  someOperation;
except
  on e: ECustomException do
    SomeCustomHandelr;
  else
     begin
       // the raise is only useful to rethrow the exception to an encompasing 
       // handler.  In this case after I have called my logger code. as Rob
       // mentioned this can be omitted if you arent handling anything because
       // the compiler will simply jump you to the next block if there is no
       // else.
       LogUnexpectedException('some operation failed',e);
       raise;
     end;
end;

Be careful that there is similar looking form without the "raise" that DOES have the side effect of eating/hiding any exceptions. practicies by very unscrupulous developers who have hopefully moved on to positions with the competition.

with ADOQuery1 do begin  
  // .. fill out sql.text, etc  
  try    
    execSQL; 
  except
    // no handler so this just eats any "errors"    
  end;

Fiacre

Mike, your first example does NOT require "raise." Any exceptions that aren't of type ECustomException will propagate to the next handler automatically. Simply omit the entire "else" section. – Saltire

You are correct. its there out of habit. the only time the else raise is needed is if yo have some other code you want to execute and then its needed. getting rusty in my old age. – Fiacre

Answer 2

Removing the except code in above code snippet will make no difference. You can (and I believe you should since it is reducing the readability) remove it.

Chlorella

Another good reason to drop code like this: when debugging, you don't want to need to step through all of these in case of an exception. – Wrist

Answer 3

This code does nothing, other than to allow the original programmer to place a breakpoint on the 'Raise' and to see the exception closer in the source to its possible cause. In that sense it a perfectly reasonable debugging technique.

Langsyne

Answer 4

Okay, really two questions here.

First, it is meaningful: if execSQL throws an exception, it's caught by the try block and forwarded to the except. Then it's forwarded on by the raise to the next higher block.

Second, is it useful? Probably not. It almost certainly is the result of one of three things:

1. Someone with pointy hair wrote a coding standard that said "all operations that can throw an exception must be in a try block."
2. Someone meant to come back and turn the exceptions made by the execSQL statment into some other, more meaningful, exception.
3. Someone new wasn't aware that what they'd written was isomorphic to letting the uter environment worry about the exception, and so thought they must forward it.

Inkstand

What worried me is that I know who wrote this code, and I do not think 1-3 apply here. Except possibly 3.. Hmm. – Brade

Oh and he's left so I can't ask him. – Brade

Did the person code in Java before? Maybe then he thought that exceptions have to be caught and did not try otherwise. – Henrieta

In Java, not all Exceptions have to caught, only "checked exceptions". – Guth

@mjustin, I think we're all agreed this is someone who has a misunderstanding of the use of exceptions. – Inkstand

Answer 5

I may have answered a bit fast, see at the end...
Like it is, it is useless for the application.
Period!

Now on the "why" side. It may be to standardize the exceptions handling if there /was/will be/is in other places/ some kind of logging code inserted before the raise:

  try
    execSQL;
  except
    // Log Exception..
    on E: Exception do
    begin
      LogTrace(Format('%s: Exception Message[%s]',[methodname, E.Message]));
      raise;
    end;
  end;

or for Cleanup code:

  try
    execSQL;
  except
    //some FreeAndNil..
    raise;
  end;

Update: There would be 1 case where I would see some use just like it is...
... to be able to put a Breakpoint on the raise line, to get a chance to see what's going on in the context on that block of code.

Prize

The second one is completely unidiomatic, one would use finally instead. Stuff like this does actually decrease the readability of the code base, so that's a reason for fixing it as well. – Autolycus

replace FreeAndNil with Connection.Rollback – Brade

Exactly Blorgbeard, any kind of housekeeping that need to be done ONLY if things go wrong, and then you move on with the Exception bubbling up – Prize

Yes, you're right. It wasn't obvious to me from the comment that the FreeAndNil() is conditional, but now I understand. – Autolycus

Seems a bit silly to use this as an opportunity to place a breakpoint, since an exception being raised sends you to the debugger anyway. – Mathias

Answer 6

Actually, I should posted this as comment to François's answers, but I don't know is it possible to insert formatted code there :( So I'm posting this as answer.

2mghie:

The second one is completely unidiomatic, one would use finally instead.

No, "finally" will cleanup object always. "Except" - only on exception. Consider the case of function, which creates, fills and return an object:

function CreateObj: TSomeObj;
begin
  Result := TSomeObj.Create;
  try
    ... // do something with Result: load data, fill props, etc.
  except
    FreeAndNil(Result); // oops: bad things happened. Free object to avoid leak.
    raise;
  end;
end;

If you put "finally" there - function will return nil always. If you omit "try" block at all - there will be resources leak in case of exception in "...".

P.S. Of course, you can use "finally" and check ExceptObj, but... isn't that ugly?

Bushweller

I agree, see my comment to Francois' answer. – Autolycus

Answer 7

This code does nothing except re-raising an exception that will allready be raised without this try except block. You can safely remove it.

Glosseme

Answer 8

The title contains quite a broad question, while its explanation gives a more specific example. So, my answering to the question as how it proceeds from the example, can doubtly add anything useful to what has already been said here.

But, maybe Blorgbeard indeed wants to know whether it is at all meaningful to try ... except raise; end. In Delphi 7, if I recollect correctly, Exit would trigger the finally part of a try-finally block (as if it were some sort of exception). Someone might consider such behaviour inappropriate for their task, and using the construction in question is quite a workaround.

Only it would still be strange to use a single raise; there, but then we should have talked about usefulness rather than meaningfulness, as Charlie has neatly observed.

Eliathas

Finally blocks execute in all cases, exceptions or not, so that can't be it. – Brade