Recent

Author Topic: Simple confirmatory question  (Read 606 times)

egsuh

  • Hero Member
  • *****
  • Posts: 1489
Simple confirmatory question
« on: October 08, 2024, 09:23:23 am »
There is a very simple function below.

Code: Pascal  [Select][+][-]
  1. function TAQFB4.RespondentEmail(tpid: string; trid: integer): string;
  2. var
  3.    xqr: TSQLQuery;
  4.    xtr: TSQLTransaction;
  5. begin
  6.    xqr := TSQLQuery.Create(nil);
  7.    xtr := TSQLTransaction.Create(xqr);
  8.    xtr.DataBase := fbAQ;
  9.    xqr.DataBase := fbAQ;
  10.    xqr.Transaction := xtr;
  11.  
  12.    try
  13.       with xqr do begin
  14.          SQL.Text := 'select Email from samplelist '
  15.                     + ' where project_id=:pid and respondent_id=:rid';
  16.          Transaction.Active := True;
  17.          Params[0].AsString := tpid;
  18.          Params[1].AsInteger := trid;
  19.          Open;
  20.          Result := Fields[0].AsString;
  21.          (Transaction as TSQLTransaction).Rollback;
  22.       end;
  23.    finally
  24.       xqr.Free;
  25.    end;
  26. end;

Here, the query and transaction are created locally. Is this customary?

I used to use TSQLQuery component dropped on the DataModule (let's say its name is SQLQuery1). But this method had risk of "nesting the same TSQLQuery and TSQLtransaction, when the function is called within another function that uses the same transaction (if not the same TSQLQuery).  So I'm moving to locally creating query and transaction components.

rvk

  • Hero Member
  • *****
  • Posts: 6575
Re: Simple confirmatory question
« Reply #1 on: October 08, 2024, 09:40:36 am »
Here, the query and transaction are created locally. Is this customary?

I used to use TSQLQuery component dropped on the DataModule (let's say its name is SQLQuery1). But this method had risk of "nesting the same TSQLQuery and TSQLtransaction, when the function is called within another function that uses the same transaction (if not the same TSQLQuery).  So I'm moving to locally creating query and transaction components.
Yes, this is customary. Precisely for the reason you mention.
Although I would make it a standalone function, passing the database, in which case it can even be used for different database connections.
But making it a function in the datamodule is also possible.

PS. I would build in some error checking. If your query doesn't return a result it will generate an exception at the moment (at the Fields[0] which doesn't exist when there is no resultset). That could be intentional but you need to handle the exception a step higher then.

PPS. I also would make such function a bit more generic. For example also passing which fieldname you want to return.
But I guess you get the point... standalone functions like this will greatly reduce the chances for errors in using the wrong TSQLQuery and messing things up.

PPPS. I also have some procedure which change the database with insert, update and delete. In that case you also need to pass a TSQLTransaction so the update is done in the correct transaction flow for the rest of your program.

egsuh

  • Hero Member
  • *****
  • Posts: 1489
Re: Simple confirmatory question
« Reply #2 on: October 08, 2024, 10:13:37 am »
@rvk

Thank you for your replay, which gives me comfort. I'll add another layer of try... except so that errors are handled (even though some parts are left intentionally so that logical errors raise exceptions.)

Regarding generic approach, I do not write many programs.

Third point is interesting. Probably I might have to define like (with default values of Transaction, I do not have to change other codes) :

Code: Pascal  [Select][+][-]
  1. function TAQFB4.RespondentEmail(tpid: string; trid: integer; Transaction: TSQLTransaction=nil): string;
  2. var
  3.     ...
  4. begin
  5.     ...
  6.     if Transaction  = nil then begin
  7.         xtr := TSQLTransaction.Create (xtr);
  8.         xtr.DataBase := fbAQ;
  9.     end
  10.     else xtr := transaction;
  11.  
  12.     xqr.Transaction := xtr;
  13.  
  14.     ...
  15. end;

Really thank you for the points. They are learnable from nowhere but experiences plus ingenuity.

Ștefan-Iulian Alecu

  • New Member
  • *
  • Posts: 21
Re: Simple confirmatory question
« Reply #3 on: October 08, 2024, 11:00:43 pm »
Along with what rvk mentioned, you should probably also guard xtr behind a try..finally block. This is how I'd approach your code in particular:

Code: Pascal  [Select][+][-]
  1. function TAQFB4.RespondentEmail(AProjectID: string; ARespondentID: integer): string;
  2. var
  3.   query: TSQLQuery;
  4.   transaction: TSQLTransaction;
  5. begin
  6.   query := TSQLQuery.Create(Self);
  7.   transaction := TSQLTransaction.Create(query);
  8.  
  9.   try
  10.     query.Database := fbAQ;
  11.     transaction.Database := fbAQ;
  12.     query.Transaction := transaction;
  13.  
  14.     query.SQL.Text := 'select Email from samplelist where project_id = :pid and respondent_id = :rid';
  15.     query.Params.ParamByName('pid').AsString := AProjectID;
  16.     query.Params.ParamByName('rid').AsInteger := ARespondentID;
  17.  
  18.     if not transaction.Active then
  19.       transaction.StartTransaction;
  20.  
  21.     try
  22.       query.Open;
  23.       if not query.EOF then
  24.         Result := query.Fields[0].AsString
  25.       else
  26.         raise Exception.Create('No email found for the specified project and respondent IDs.');
  27.  
  28.       transaction.Commit;
  29.     except
  30.       on E: Exception do
  31.       begin
  32.         transaction.Rollback;
  33.         raise Exception.Create('Error fetching respondent email: ' + E.Message);
  34.       end;
  35.     end;
  36.   finally
  37.     query.Free;
  38.   end;
  39. end;
  40.  

CharlyTango

  • Jr. Member
  • **
  • Posts: 90
Re: Simple confirmatory question
« Reply #4 on: October 09, 2024, 01:49:55 pm »
Don't create zombies and also free the transaction, but I guess that was just a typo anyway.

Code: Pascal  [Select][+][-]
  1. ....
  2.   finally
  3.     transaction.Free
  4.     query.Free;
  5.   end;

But the real question beside a fomal query strategy in a function is to have some kind of business logic.
Is it worth an exception if the query for a mail address brings no result?

This would mean that a serious error has occurred that is incompatible with the further course of the programme, which in my opinion is not the case here. Therefore an exception might be the wrong tool.
Lazarus stable, Win32/64

Zvoni

  • Hero Member
  • *****
  • Posts: 2738
Re: Simple confirmatory question
« Reply #5 on: October 09, 2024, 02:17:10 pm »
But the real question beside a fomal query strategy in a function is to have some kind of business logic.
Is it worth an exception if the query for a mail address brings no result?

This would mean that a serious error has occurred that is incompatible with the further course of the programme, which in my opinion is not the case here. Therefore an exception might be the wrong tool.
Agreed.
A Try/Finally/Except for a simple SELECT-Query doesn't make sense, except if you are allowing Users to type the Query themselves!
Then it does make sense to guard against TYPO's

If the Query itself (the SQL-Statement) is correct, then it's a simple check against EOF/RecordCount>0

Next: For SELECT-Queries Transactions are superflous. You cannot commit them, you cannot roll them back.
This only applies to INSERT/UPDATE/DELETE
One System to rule them all, One Code to find them,
One IDE to bring them all, and to the Framework bind them,
in the Land of Redmond, where the Windows lie
---------------------------------------------------------------------
Code is like a joke: If you have to explain it, it's bad

rvk

  • Hero Member
  • *****
  • Posts: 6575
Re: Simple confirmatory question
« Reply #6 on: October 09, 2024, 02:19:50 pm »
Don't create zombies and also free the transaction, but I guess that was just a typo anyway.
No no no..  %) Look closer at the source.
TSQLTransaction is created with xtr (the query itself) as Owner.

NEVER free components of which your code isn't itself the owner (by having it created with nil).

So either use Create(nil) and do Free OR do Create(Owner) and let the owner free the object when doing Owner.Free.

Is it worth an exception if the query for a mail address brings no result?
Yes, I would return an empty string in this case (not an exception).


 

TinyPortal © 2005-2018