I maintain and work on a number of legacy code applications. One of them has earned itself some refactoring. So I’ll try to share some of the changes I’ve been making to improve it. Today, a problem that’s bothered me for a while: the DB class connects to the database even if no queries are being used.
I should be thankful that this legacy app doesn’t have a bunch of mysqli_* calls all over the place. Back when I wrote it I at least had the presence of mind to wrap SQL functions in an abstraction class. So this change was quite easy to make.
Removing redundant connections
The problem was that the database connection was called in the config file which sets the connection parameters then connects. But… what about all those pages without queries? What about the search bots that hit the front of the site without logging in? Every page view was connecting to the database. The solution was simple.
The connection call in the config file was removed.
Each query function in the abstraction class calls the connect method, which itself checks if a connection exists, and if not, connects:
if( isset($this->dbc) && is_a($this->dbc, 'mysqli') && !$this->dbc->connect_errno ){ return true; } $this->dbc = new mysqli(....);
Retrying Failed Database Connections
Another improvement I made while I was in the DB class anyway, was to have the connect function retry a few times if a connection fails. This was handled with a basic recursive call to the connect function.
public function connect($retries=0) { // ... check if connected. If not, connect. if( !isset($this->dbc) || !is_a($this->dbc, 'mysqli') || $this->dbc->connect_errno ){ if( $retries < 3 ){ $this->disconnect(); // Just in case. sleep(1); return $this->connect($retries+1); } // ... handle error. return false; }else{ return true; }
Conclusion
Refactoring a legacy application doesn’t need to be a major undertaking. Even fixing old headaches or making tiny improvements can help make bad code good less bad.