Jump to content
Ian Branch

Cleaning up some Exception handling...

Recommended Posts

Hi Team,

I am cleaning up some exception handling in some old code.

The code goes something like the following..

if E:ErrorCode = xx then
	doshomething
else if E:ErrorCode = yy then
	dosomethingelse
...
..
else
	TaskMessageDlg('Errormessage!', E.Message, mtError, [mbOK], 0);

If using MadExcept or EurekaLog, is the following proper/acceptable??

if E:ErrorCode = xx then
	doshomething
else if E:ErrorCode = yy then
	dosomethingelse
...
..
else
	raise;

to let MadExcept or EurekaLog process the unhandled exception(s)??

 

Regards & TIA,

Ian

Share this post


Link to post
Guest

dont delegate your duty to others!

You run the serious risk of liking it and becoming a pretty lazybones.

it's duty of programmer take care of your code.

this tools it's so good and big help when learning, but find concrete solutions will make you a more confident programmer for complex cases and situations.

 

now, about use of exceptions, prefer:

try

...

except

On E:Exception do  (level more High)

...

On E:Exception do  (level more Low)

end;

 

High = sub-Exception

Low = Ancestral Exception

 

you can use "IF/CASE" but it's not recommendable.

Edited by Guest

Share this post


Link to post

Hi joaodanet2018,

Thank you for your input.

My example above was extremely simplified and was specifically enquiring if the use of raise in this context was correct.

The actual code is handling the common issues and giving the User an opportunity to correct their mistakes, however there are always the exceptions, pun intended, that aren't accounted for.  MadExcept & EurekaLog allow you to capture details of this exceptions and look at root cause with the objective of eliminating or mitigating them.

 

Ian

Share this post


Link to post
1 hour ago, Ian Branch said:

The code goes something like the following..

E:ErrorCode is not valid syntax, did you mean E.ErrorCode?

Quote

If using MadExcept or EurekaLog, is the following proper/acceptable??

to let MadExcept or EurekaLog process the unhandled exception(s)??

The original code displays a TaskMessageDlg on an unhandled exception, but regardless of the type of exception raised, any code following the try..except block continues to run normally,  With your proposed change, on a re-raise of an unhandled exception, any code following the try..except block won't run anymore.  That is a change in program flow, not just a change in error reporting. Is that what you really want?

Edited by Remy Lebeau

Share this post


Link to post

Hi Team,

I do appreciate your inputs.

I thought I was keeping it simple and asking a simple question...  Oh well.

OK.  Here is the full code sequence....Users is a TTable.

procedure TMainForm.UsersEditError(DataSet: TDataSet; E: EDatabaseError; var Action: TDataAction);
begin
  //
  _EditError(Action, E, DataSet);
  //
end;
procedure TMainForm._EditError(out Action: TDataAction; E: EDatabaseError; DataSet: TDataSet);
var
  sMessage: string;
begin
  //
  sMessage := 'The record you are trying to edit in the ' + DataSet.Name + ' table ';
  //
  Action := daAbort;
  //
  if (E is EEDBError) then
  begin
    //
    case EEDBError(E).ErrorCode of
      EDB_ERROR_LOCKROW:
        if TaskMessageDlg('Locked record!', sMessage + ' is currently locked, do you want to try to edit this record again?', mtWarning, [mbYes, mbNo], 0) = mrYes
        then
          Action := daRetry;
      EDB_ERROR_ROWDELETED:
        begin
          TaskMessageDlg('Deleted record!', sMessage + ' has been deleted since it was last retrieved', mtError, [mbOK], 0);
          DataSet.Refresh;
        end;
      EDB_ERROR_ROWMODIFIED:
        begin
          TaskMessageDlg('Modified record!', sMessage + ' has been modified since it was last retrieved, the record will now be refreshed', mtWarning,
            [mbOK], 0);
          DataSet.Refresh;
          Action := daRetry;
        end;
    else
      TaskMessageDlg('Errormessage!', E.Message, mtError, [mbOK], 0);
    end;
    //
  end
  else
    TaskMessageDlg('Error message!', E.Message, mtError, [mbOK], 0);
  //
end;

Why have I gone this route?  This way I can use one _EditError procedure for all TTable or TQuery EditErrors. 

My basic question is - Is it safe/correct/appropriate to replace the last two 'TaskMessageDlg(...' with 'raise;' to pass whatever the unhandled exception is to MadExcept or EurekaLog?

 

Regards,

Ian

Share this post


Link to post
Guest

my english it's not so good, ok?

I think that your "work" is so much than necessary for your task!

Look, the Embarcadero classes is projected to abord many situations.

Take a look at TDataset component events, you can extract many info about the "errors / exceptions" basic.

Now you want to show for your users more details about some situations not valid etc... it's ok.

but dont mix your code with language flow.. let it work for you.

for example, you use:

 _EditError(Action, E, DataSet);  to call...
TMainForm._EditError(out Action: TDataAction; E: EDatabaseError; DataSet: TDataSet);

but 

TMainForm.UsersEditError(DataSet: TDataSet; E: EDatabaseError; var Action: TDataAction);

wait for a "ACTION" return...  but you wait user choice any one (or press OK) using

TaskMessageDlg

is it not very so much? 

 

remember: in events like these, basically, YOU DONT use a modal window or wait for user answer! Why? because you can break all natural flow projected for these events-type!

When an...

Action: TDataAction

events like these wait that you "just" give its value:  ABORT? RETRY? CANCEL?  -- you see?

then, out these events, you can inform your user that some error occurred, not into it. -- you see?

 

then you can try some like this:

 

... // let save our data on table...

try
  tbOrders.post;
except
 // using E:Exception (low level ) then any other exception dont will be showed...for that, you can use verify any other sub-exception at first
 on E:Exception do // here you can use a user-procedure to details the error etc...
   ShowMessage('Hey user, this exception was raised')
end;

on event-handler from tbOrders you can do it this: (here showing a DLG into event it's ok, because the flow is wait any action - you see?)
begin
// DlgMessage return an "integer / modalresult" // Action wait for an "TDataAction" --> needs a "casting here"
  Action := DlgMessage( E.Message ... Dataset.Fields... ...'Retry?', [yes, no]); // yes=daRetry,  no=daAbort
  // if Action=caRetry, then the Dataset class (itself) will go try again!!!
end;
Edited by Guest

Share this post


Link to post

So.  The short answer is No.

Raise has to be within the exception block.

That was all I need to know.

 

What I can do is replace this..

    else
      TaskMessageDlg('Errormessage!', E.Message, mtError, [mbOK], 0);
    end;
    //
  end
  else
    TaskMessageDlg('Error message!', E.Message, mtError, [mbOK], 0);
  //
end;

with this..

    else
      Action := daFail;
    end;
    //
  end
  else
    Action := daFail;
  //
end;

The solution I was ultimately after.

Thank you for your contributions.

 

Ian

Edited by Ian Branch

Share this post


Link to post
Guest

you can use "raise;" to re-pass the last exception raised to next level in the calls...

 

button1 call  procHello... calll procHiiiii

 

now, an exception occurrs in procHiiii

you want catch it on next level ( procHello )

then in procHiiii you use:

   raise;

 

that way, next levels (any one) should treat it, else the Application receive a memory leak, Av or other messages. .. because the exception was not treated in any level. you see?

 

Share this post


Link to post
Guest

When you show the code above... I cannot see that you are inside an exception handler.

EDatabaseError is an exception class, yes, but it looks like you are inside an event handler.

A class inherited from Exception can be instantiated (created) without being raised.

If you have the sources, put a breakpoint there and click you way down the stackframe, you'll reach the code where the handler is called, you can see exactly what happens there.

You can raise exceptions from an event handler, sure. In that case i would write something like "raise EDatabaseError.Create(....)". I do not think this is correct in your case. And i do not think MadExcept will log it because it is not external. But that i would have to test.

If you are in the event handler (the docwiki is down, so i cannot check properly,my guess) then you have the TDataAction argument (parameter) where you can tell the DAC how to proceed. I must check the docs but i would guess when you assign the event, an exception is not raised, instead you write to Action. IIRC if no handler is assigned an exception is raised from the DAC instead (must check though - the best way if one have sources and put a breakpoint IMHO).

 

I must say that i was never a fan of how this is handled in TDataSet, not enough info and flexibility, but that is another discussion.

Share this post


Link to post

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

×