Handling DB transaction in closure

Question

I think DB transaction should be as small as possible, and close it soon.

So I wrote a helper function like 'using' statement in C#.

function transaction($conn, $closure) {
 $conn->beginTransaction();
 try {
 $closure($conn);
 $conn->commit();
 } catch (Exception $e) {
 $conn->rollback();
 throw $e;
 }
}
$conn = new PDO('mysql:....');
$conn->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
transaction($conn, function($conn) {
 $conn->query('insert ....');
 $conn->query('insert ....');
});

but with this pattern:

• I have to capture manually any variables with use()
• Stack trace will be bit complicated

or, should I write try/catch and begin/commit in every transaction?

• Phrancis: Welcome to Code Review! I think you should definitely begin and commit transactions when interacting with the database, instead of running it in auto-commit mode. I hope you get some great answers!

Answer 1

There's a couple of things I'd recommend you change about your current code, and there's an alternative to your using a closure's use construct to pass in other variables.

The first thing I'd do, is to change your function's signatures to type-hint the expected types for each argument, ie change the transaction function to look something more like this:

function transaction(PDO $conn, callable $closure)
{}

Optionally, replacing the callable type-hint with Closure. The upside of callable is that old-school callable constructs will be accepted, too (array($object, 'someMethod') or even ucfirst), the downside is, of course that functions like ucfirst are accepted, but clearly not a valid function in this context.

Anyway, apart from using use to pass in additional variables, you could add a third argument to the transaction function, and type-hint an array:

function transaction(PDO $conn, callable $closure, array $arguments = [])
{}

Then, simply invoke the $closure argument using call_user_func_array to pass the arguments that need to be passed:

array_unshift($conn, $arguments);//prepend $conn to the arguments 
call_user_func_array($closure, $arguments);

And you're there... Of course, if you find that array_unshift call clumsy looking (which it kind of is), you can just assume the $arguments array to contain the PDO connection from the off, in which case:

function transaction(callable $callback, array $arguments)
{
 if (!$arguments[0] instanceof PDO) {
 throw new InvalidArgumentException('arguments should contain PDO instance');
 }
 $conn = $arguments[0];
 try {
 $conn->beginTransaction();
 call_user_func_array($callback, $arguments);
 $conn->commit();
 } catch (Exception $e) {
 $conn->rollBack();
 throw $e;
 }
}

Again, there is a downside to this approach, the main disadvantages, IMO, are:

• Overhead (a PHP closure is syntactic sugar, it's actually creating a new instance of the Closure class.
• The $arguments array will often be a bit too complex for its own good, leaving you with code that actually calls your transaction function to decide on the actual logic -> every call can look different, and often will contain SQL strings, fetch calls and the like. Ugly code is harder to maintain...
• Like you said: the stack trace looses some of its relevance, making debugging harder.

The last two issues (code that is harder to maintain + harder to debug) are, to me, severe problems, so much so, that I'd recommend not to use the transaction function, but instead write the try-catch blocks where and when I need them.

• unarist: Thanks. I didn't think an array parameter instead of use(), it's interesting! However, it seems to be tricky and usage is like as use(), so I'd prefer use().