Simple confirmatory question

[egsuh]

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]

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]

@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]

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]

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.

[Zvoni]

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

[rvk]

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).