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?

Was it helpful?

Solution

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;

OTHER TIPS

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.

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.

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.

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.

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?

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.

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

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top