propelorm/Propel2 — Issue #68: [RFC] Removing debug stuff in ConnectionWrapper class
Author: hhamon
Created: 2011-11-22 10:06:57+00:00
State: open

Hi,

I looked at the Connection namespace and I have some suggestions on how to improve these classes. First of all, I'm a bit suprised by the number of classes to handle connection. Maybe we could remove some of them, but anyway this is not the topic of this RFC.

The ConnectionWrapper class does too much thing and I suggest to remove all calls related to debug. It will simplify the code a lot and it will certainly add a small speed boost in production as they won't have any conditions on the isDebug property.

What I suggest is to keep the ConnectionWrapper as simple as possible by removing all debugging stuff and overriding methods in an AdvancedConnectionWrapper or DebugConnectionWrapper or ProfiledConnectionWrapper class (which actually corresponds to the DebugPDO class).

We have two ways of doing this: inheritance or decoration with composition.

Inheritance way:

class ConnectionWrapper
{
public function exec($sql)
{
return $this->connection->exec($sql);
}
}

class ProfiledConnectionWrapper extends ConnectionWrapper
{
public function exec($sql)
{
$debug = $this->getDebugSnapshot();

$return = parent::exec($sql);

$this->log($sql, null, 'exec', $debug);
$this->setLastExecutedQuery($sql);
$this->incrementQueryCount();

return $return;
}
}

So in production we just need to instanciate the ConnectionWrapper class or use the other in development mode:

// Production
$conn = new ConnectionWrapper(new \PDO());

// Development
$conn = new ProfiledConnectionWrapper(new \PDO(), array(
'foo' => 'bar' // Options to configure what we want to profile
));

This ProfiledConnectionWrapper connection will also be able to receive an injected Monolog dependency, so that the main ConnectionWrapper keeps remain free of any debug things.

Or we can do it by composition:

class ProfiledConnectionWrapper implements ConnectionWrapperInterface
{
private $conn;

public function __construct(ConnectionWrapperInterface $conn)
{
$this->conn = $conn;
}

public function exec($sql)
{
$debug = $this->getDebugSnapshot();

$return = $this->conn->exec($sql);

$this->log($sql, null, 'exec', $debug);
$this->setLastExecutedQuery($sql);
$this->incrementQueryCount();

return $return;
}
}

So, finally we would have the following code:

// Production
$conn = new ConnectionWrapper(new \PDO());

// Development
$conn = new ProfiledConnectionWrapper(new ConnectionWrapper(new \PDO()), array(
'foo' => 'bar' // Options to configure what we want to profile
));

What do you think about this RFC?

Cheers.