php – Proper use of commit and rollback

Does this appear as proper use of the commit and rollback functions?

I honestly haven’t used PDO functions directly – typically used a framework like Laravel or similar.

I do agree with the advice on and it has a page about PDO usage with a section about Transactions. It mentions:

Please note the following important things:

  • PDO error reporting mode should be set to PDO::ERRMODE_EXCEPTION
  • you have catch an Exception, not PDOException, as it doesn’t matter what
    particular exception aborted the execution.
  • you should re-throw an
    exception after rollback, to be notified of the problem the usual way.
  • also make sure that a table engine supports transactions (i.e. for
    Mysql it should be InnoDB, not MyISAM)
  • there are no Data definition
    language (DDL) statements that define or modify database schema among
    queries in your transaction, as such a query will cause an implicit

It appears that the error reporting mode is set to PDO::ERRMODE_EXCEPTION✅ , though the catch only catches PDOException instances and the exception isn’t re-thrown.

What, if anything, can I do to improve this code?

Let’s look at these three lines near the top:

$addname = isset($value('addname ')) ? $value('addname ') : '';
$addemail = isset($value('addemail')) ? $value('addemail') : '';
$addcomment = isset($value('addcomment')) ? $value('addcomment') : '';

Should the first string index contain a space – 'addname '? Perhaps that is a typo?

The server is running PHP 7.3 or higher, right? Hopefully that is the case since at the time of writing those are the supported versions receiving Security fixes.

Presuming that is the case, then these lines could be simplified using the null-coalescing operator i.e. ??

$addname = $value('addname ') ?? '';
$addemail = $value('addemail') ?? '';
$addcomment = $value('addcomment') ?? '';

If the first two are actually '' then does it even make sense to run the queries? If not, then handle that case appropriately – e.g. with an error message or other means.

Next, let’s look at that first query:

$selectCheck = $dbc->prepare("SELECT * FROM mainTable WHERE `name` = :newname AND `email` = :newemail");

Since that query is only used later to call $selectCheck->rowCount() then there isn’t a point to select *. It could simply select 1 or a single field. This will avoid the database query returning data that isn’t needed.