In Delphi ExecSQL succeeded or failed

Hi I have this function to update my access database with TUniQuery:

var
  Res:Boolean;
begin
Res:=false;
  try
  with MyQuery do
  begin
    Active := false;
    SQL.Clear;
    SQL.Add('Update MYTABLE');
    SQL.Add('set username='+QuotedStr(NewUserName));
    SQL.Add(',password='+QuotedStr(NewPassword));
    SQL.Add('where username='+QuotedStr(ACurrentUserName));
    ExecSQL;
    Res:=true;
  end;
  except
  Res:=False;
  end ;
  Result:=Res;
end;

      

Is Try ... other than enough to KNOW when "ExecSQL" succeeds or fails?

or is there any other better approach?

Thank you

+3


source to share


1 answer


You might want your update to succeed if no exceptions were made. This means the database is responsive and parsed and executed by your statement without syntax errors.

In a statement, as shown, you can also be sure that exactly one row was updated , because I guess that is your intention.

To check this, you can resort to the result of the ExecSQL method, which returns the number of rows affected by the execution of the statement. So, you can change your code to this:

begin
  with MyQuery do
  begin
    Active := false;
    SQL.Clear;
    SQL.Add('Update MYTABLE');
    SQL.Add('set username='+QuotedStr(NewUserName));
    SQL.Add(',password='+QuotedStr(NewPassword));
    SQL.Add('where username='+QuotedStr(ACurrentUserName));
    Result := ExecSQL = 1; //exactly 1 row updated
  end;
end;

      

I also changed the unconditional exception handler, as it might not be the proper site to have any exception, and also removed the local variable to save the result as it really isn't necessary.

After reading the added text and reconsidering your question:

Is Try ... other than enough to KNOW when "ExecSQL" succeeds or fails?



You really need to change your mind about exception handling and return the boolean from this procedure. Exceptions were introduced as a completely new concept of how to deal with exceptions and errors in your programs, but you kill this new (and best of all) approach and resort to the old way of returning a value indicating success or failure.

In particular, a block try/exception

that eats any exception is bad practice as you will be killing exceptions that could be raised for too many reasons: out of memory, network problems like connection lost in the database, etc.

You need to rethink your approach and handle these exceptions or errors at the appropriate level in your application.

My advice:

  • change this from function to procedure, new contract: it only returns if it is executed, otherwise an exception is thrown.
  • If an exception occurs, let it fly out of the subroutine and handle this situation elsewhere
  • throw your own exception if exactly 1 row is not updated.
  • change the query to use parameters (avoiding sql injection)

The procedure might look like this:

procedure TMySecurityManager.ChangeUserNameAndPassword();
begin
  MyQuery.SQL.Text := 'Update MYTABLE'
              + '   set username = :NewUserName'
              + '       , password = :NewPassword'
              + ' where username = :username';
  MyQuery.Params.ParamByName('NewUserName').AsString := NewUserName;
  MyQuery.Params.ParamByName('NewPassword').AsString := NewPassword;
  MyQuery.Params.ParamByName('username').AsString := ACurrentUserName;
  if MyQuery.ExecSQL <> 1 then
      raise EUpdateFailed.Create('A internal error occurred while updating the username and password');
  //EUpdateFailed is a hypotetical exception class you defined.
end;

      

+9


source







All Articles