HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Handling DB transaction in closure

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
handlingclosuretransaction

Problem

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?

Solution

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.

Code Snippets

function transaction(PDO $conn, callable $closure)
{}
function transaction(PDO $conn, callable $closure, array $arguments = [])
{}
array_unshift($conn, $arguments);//prepend $conn to the arguments 
call_user_func_array($closure, $arguments);
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;
    }
}

Context

StackExchange Code Review Q#90605, answer score: 3

Revisions (0)

No revisions yet.