Need Critics On This OOP Code For MySQL Transactional

[kairog]

Hello PHP friends,

I am very new to OOP and transactional database and I'm just starting to apply them to some of my coding.

I appreciate if you could comment or tell me how I can improve my code below.

Here's my code....

class myTransaction {
	
	var $connection;
	
	function __construct($dbconnection) {           
		$this->connection = $dbconnection;
                @mysql_query("SET AUTOCOMMIT=0",$this->connection);         
        }  

	function begin(){
		@mysql_query("BEGIN", $this->connection);
	}
	
	function commit() {
		@mysql_query("COMMIT", $this->connection);
		@mysql_query("SET AUTOCOMMIT=1",$this->connection);
	}
	
	function rollback()	{
		@mysql_query("ROLLBACK", $this->connection);
		@mysql_query("SET AUTOCOMMIT=1",$this->connection);
	}
	
	/*
	 * use FOR UPDATE at the end of SELECT statement
	 * or use LOCK IN SHARE MODE at the end of SELECT statement
	 */
	function query($query, $lock_mode=''){
		print $query;
		if ($result = @mysql_query($query . $lock_mode )){
			return $result;
		} else {
			return false;
		}
	}
	
}

and here's how I apply it...

$connection = mysql_pconnect($dhost,$duser,$dpass) or die ('unable to connect');
mysql_select_db($dname) or die ('unable to select database');

$new_entry = new myTransaction($connection);
$error = 0;

$new_entry->begin();

// query #1
if ($result = @mysql_query('insert....')){
} else {
     $error++;
}

// query #2
if ($result = @mysql_query('insert....')){
} else {
     $error++;
}

// query #3
if ($result = @mysql_query('insert....')){
} else {
     $error++;
}

if ($error){
     $new_entry->rollback();
} else {
     $new_entry->commit();
}

Any comment will be highly appreciated.

Cheers,

[Fou-Lu]

It has a long way to go before it becomes functional in the big picture. One of the primary purposes of OOP is to allow reusability, and a close second on that is to allow a great deal of abstraction. What you have is locked both into mysql and in both procedural and OO code.
MyTransaction should be an interface, and a potential abstract would be placed between the OOP driver and the interface. You then create factories to construct the necessary classes for you. I have chosen to un-name the createTransaction as it should be inferred from the type of driver in use for the Database::getInstance class. The MyTransaction implementation should also extend an interface for the Database, so that each transaction can be treated as a Database type.
The idea is to completely centralize the code, so that all of your second block would become this:

PHP Code:

$connection = Database::getInstance('MySQL');
$connection->open($dhost,$duser,$dpass, $dbname);

$new_entry = $connection->createTransaction();
$error = 0;

$new_entry->begin();

// query #1
if ($result = $new_entry->query(...)){
} else {
     $error++;
}

// query #2
if ($result = $new_entry->query('insert....')){
} else {
     $error++;
}

// query #3
if ($result = $new_entry->query('insert....')){
} else {
     $error++;
}

if ($error){
     $new_entry->rollback();
} else {
     $new_entry->commit();
} 


Keeping control at an aggregate level of the class will allow you to write transaction drivers for databases that do not support them (it just takes more work).

Now the only thing that needs to change to work on other databases is the use of the factory to get the instance type. Of course, you'd write the drivers to access sqlserver or oracle for example, but usage wise only the 'mySQL' would change to something like 'SQLServer' - no other usage code would need to change.

Proper OO development does take a lot of work, but the reward of re-usability and portability is well worth the effort.

[kairog]

That's great Fou-Lu. As always, you're such a great help. I'll be working on improving the class based on your suggestions. Thanks a lot!

[Dormilich]

Originally Posted by Fou-Lu

PHP Code:

$connection = Database::getInstance('MySQL');
$connection->open($dhost,$duser,$dpass, $dbname);

$new_entry = $connection->createTransaction();
$error = 0;

$new_entry->begin();

// query #1
if ($result = $new_entry->query(...)){
} else {
     $error++;
}

// query #2
if ($result = $new_entry->query('insert....')){
} else {
     $error++;
}

// query #3
if ($result = $new_entry->query('insert....')){
} else {
     $error++;
}

if ($error){
     $new_entry->rollback();
} else {
     $new_entry->commit();
} 


looks like PDO code …