Learning OO PHP: bad deisgn or bad practice?

[zenlord]

Hi,

 

I have a working PHP app with limited functionality that I use daily. Some months ago I wanted to add functionality, but soon came to a conclusion I should convert my procedural code to OO PHP. So I read some books by David Powers and Matt Zandstra (the latter was sometimes a little bit too complex for me) and started converting.

 

I now have converted some parts of my app and created new functionality and everything was working perfectly, until I started to use methods from class A inside methods of class B. I started getting irrational errors: sometimes it worked, sometimes it returned an error.

 

I think I have narrowed it down to the use of __construct() and __destruct() in all of my classes, and I'm wondering what is the (better/best) solution. My classes at the moment are all built the same way:

 

class A {
function __construct() {
$this->_conn = @ pg_connect( "host=localhost dbname=* user=* password=*" );
}

function get_detail() {
$this->_res = @ pg_query($this->_conn, "SELECT detail FROM table WHERE ph_id='$id'" );
}

function __destruct() {
@ pg_free_result( $this->_res );
@ pg_close( $this->_conn );}
}

I thought I was doing the correct thing by instead of repeating the setting of $this->_conn inside every method, setting that variable upon instantiating the object.

 

But now, I have to make a $this->_conn2 and so on to circumvent the irrational errors (always the same: '5 is not a valid result resource', where '5' should be the '$this->_conn'.

 

Is this a bad practice, or am I doing something else wrong? I also tried 'unset( $this->_conn )' after calling the method, but that didn't seem to work.

 

I'm thinking of looking for a generic PostgreSQL-class so that I can establish the connection inside every method itself, rather than in the __construct() - do you think that would solve my problem?

 

This might be an easy question for someone who is used to OO PHP, so I'm hoping to get an answer which helps me understand the problem. THX for all the help I can get!

[BlueSkyIS]

i don't have an answer in regard to oop, but all of those @'s should be deleted.

[zenlord]

You're right that that could make it easier to debug, but I don't think those are the problem, since they only suppress error output and leave it to my error handling (which is in place, albeit very rudimentary).

 

I will try it and see how it goes.

 

(ah, and I forgot to mention: the reason why I'm not trying the solutions I'm thinking of instead of asking here is that I really would like to be sure that I'm doing 'The Right Thing' - I want to add lots of functionality to this app, and starting of on the wrong foot is a recipe for lots of headaches...)

 

V

[zenlord]

I will try it and see how it goes.

That didn't solve the problem...

 

V

[ChemicalBliss]

I would definitely advise against doing any more serious conversions until you really understand what it is your trying to do.

 

By that I mean, OOP is just a way of abstracting code, to make it easier for multiple developers to work and update the same project without interfering with each other. It also generally makes code a little easier to read/debug/modify and generally work with.

 

I see you have tried to abstract your database functionality.

Your class structure has no problems with it that I can see.

How are you loading the classes and using them. Give us the smallest example you can provide that reproduces the problem (preferably every time).

[KevinM1]

Trying to do a 1:1 conversion from procedural to OOP won't work.  The two methodologies just don't translate neatly into one another.

 

Remember that objects can contain other objects.  Instead of each class handling everything from instantiating a db, running queries on it, performing logic based on those queries, and destroying/closing/cleaning up the db afterward, you need to break it up and have at least one generalized db class that the others can reference, or contain a reference to.  You should look into PDO, which is a generalized OO wrapper for a multitude of dbs (pdo, http://www.php.net/manual/en/intro.pdo.php) before wasting time reinventing the wheel.

 

For the rest, go over the patterns again, especially the instantiation and structural patterns again.

[zenlord]

First off, I would like to say that I'm very cautious in 'converting' / translating the app to OOP: *everytime* I add new functionality, it is tested and if it doesn't work, I can easily go back to the working code. It is not copy/paste-job, except for a small amount of functions - I am using this conversion to make the best of my app, because most of the procedural code is 3 to 5 years old and I guess I have learned some things in the meanwhile

 

How are you loading the classes and using them. Give us the smallest example you can provide that reproduces the problem (preferably every time).

Well, this is how I'm doing it now, and how it is working:

	$Us		= new User;
	$userid	= $Us->get_detail_by_uname( $_SESSION["username"], "proius_id" );
	$Do4 	= new Dossier;
	if ( $Do4->category( $fileNr ) === 1 ) {
		$Pr2 = new Prestatie;
		$Pr2->mk_new_pc( $open, "Aanmaak dossier", 1, $userid, $fileNr, 6);
	}
	$Pr3 = new Prestatie;
	$Pr3->mk_new_ph( $open, "Aanmaak dossier", 10, $userid, $fileNr, 0);
	unset ($Do4);
	unset ($Pr2);
	unset ($Pr3);
	unset ($Us);

In all the examples I read about, it was possible to do a very simple:

$U = new User;

$Ua = $U->get_detail('a');

$Ub = $U->get_detail('b');

$D = new Dossier;

$D->set_detail($Ua, $Ub);

(for example).

 

If I combine two classes in a function/method like this, I get errors that the dbase-connection is not a valid result resource, probably because I use $_conn in every class as the property to contain the dbase-link. Since I started combining classes, I have needed to instantiate new objects for every new method (in the above example I would have to instantiate a new object $U1 for the method $U1->get_detail('b'), or it wouldn't work.

 

The unsetting of the variables is also something I was not doing until I was having problems.

 

2 options:

* or, I'm doing nothing wrong and this is just how it's done,

* or, I did read the books correctly, but I don't know how to make certain design choices and suffer from the consequences.

 

Anyhow: I will read up on PDO - I know I have it installed on my server because davical needs it for my caldav server.

 

Thank you for your concerns about me doing a lot of damage to my (working) app. I will report back tomorrow evening if PDO has helped me out or not.

 

Vincent

[zenlord]

Since I won't be able to work on my app until tomorrow evening, but I couldn't refrain myself from looking up some stuff, I'm posting this for my own convenience (and maybe someone can tell if this is a good idea or not before I start it).

 

I found info about PDO, and it looks like it's the way to go. I even found a PDO-session-class (http://www.walterebert.com/code/session-pdo.html), which is something I tried to build myself (Postgresql session handler), but I failed miserably. The PDO-session-class instantiates a persistent connection to the dbase, in a way I should be able to re-use the connection inside every class until the session is ended.

 

The PDO-docs on php.net describe 'prepared statements', something I had never heard about, in a way that it looks very interesting to use myself, since my app is merely a frontend to an extensive dbase, and uses a lot of dbase-transactions.

 

To do these kind of changes, I will work on a copy of the code and dbase and test it extensively before moving to this class exclusively.

 

I'm open to any opinions, but feel good about the info the answers in this thread have lead me to! THX!

 

Vincent

[ChemicalBliss]

First off, I would like to say that I'm very cautious in 'converting' / translating the app to OOP: *everytime* I add new functionality, it is tested and if it doesn't work, I can easily go back to the working code. It is not copy/paste-job, except for a small amount of functions - I am using this conversion to make the best of my app, because most of the procedural code is 3 to 5 years old and I guess I have learned some things in the meanwhile

 

How are you loading the classes and using them. Give us the smallest example you can provide that reproduces the problem (preferably every time).

Well, this is how I'm doing it now, and how it is working:

	$Us		= new User;
	$userid	= $Us->get_detail_by_uname( $_SESSION["username"], "proius_id" );
	$Do4 	= new Dossier;
	if ( $Do4->category( $fileNr ) === 1 ) {
		$Pr2 = new Prestatie;
		$Pr2->mk_new_pc( $open, "Aanmaak dossier", 1, $userid, $fileNr, 6);
	}
	$Pr3 = new Prestatie;
	$Pr3->mk_new_ph( $open, "Aanmaak dossier", 10, $userid, $fileNr, 0);
	unset ($Do4);
	unset ($Pr2);
	unset ($Pr3);
	unset ($Us);

In all the examples I read about, it was possible to do a very simple:

$U = new User;

$Ua = $U->get_detail('a');

$Ub = $U->get_detail('b');

$D = new Dossier;

$D->set_detail($Ua, $Ub);

(for example).

 

If I combine two classes in a function/method like this, I get errors that the dbase-connection is not a valid result resource, probably because I use $_conn in every class as the property to contain the dbase-link. Since I started combining classes, I have needed to instantiate new objects for every new method (in the above example I would have to instantiate a new object $U1 for the method $U1->get_detail('b'), or it wouldn't work.

 

The unsetting of the variables is also something I was not doing until I was having problems.

 

2 options:

* or, I'm doing nothing wrong and this is just how it's done,

* or, I did read the books correctly, but I don't know how to make certain design choices and suffer from the consequences.

 

Anyhow: I will read up on PDO - I know I have it installed on my server because davical needs it for my caldav server.

 

Thank you for your concerns about me doing a lot of damage to my (working) app. I will report back tomorrow evening if PDO has helped me out or not.

 

Vincent

 

What you would do is abstract each part of your code so they they can work on their own, each class should technically, not require any other class except its "dependencies" which usually is just the database layer.

 

Your index.php would basically house your "core" for that specific project. It would load up all the classes and files necesary to run.

It would then call/set any objects it needs to use, then use them in the order required. When you instantiate a new class it should only be (for basic use) in this index file and as so you can have a single reference to the database layer. You simply pass this database object to any other object that needs it via its instantiating parameters (its __construct() ), that object then saves the DB Layer in it's variable scope (usually private).

 

eg:

<?php

Class A {

private $db_layer;

public function __construct($database_object){
	$this->$db_layer = $database_object;
}

public function use_db_somehow(){
	$this->db_layer->some_db_call();
}
}

Class B {

private $db_layer;

public function __construct($database_object){
	$this->$db_layer = $database_object;
}

public function use_db_somehow(){
	$this->db_layer->some_db_call();
}
}

// index
$db = new Database_Layer_Object();
$a = new A($db);
$b = new B($db);
$a->use_db_somehow();
$b->use_db_somehow();

?>

 

hope this helps

[beegro]

Ah yes, PDOs!  I think they are the best way to go when developing OO applications.  It is a simple series of objects (PDO connections, statements, errors, etc) that simplify what a lot of us use to build custom abstraction layers to do.  I highly recommend using the PDOs whenever possible.  However, I made the full plunge into prepared statements on one project only to find some serious issues that I didn't consider when I started. 

 

When using Postgres, the query engine for prepared statements is different then the engine used to process a query string passed directly through the PDOs query method.  In a couple of cases the database engine took 20+ seconds longer to complete the prepared statement vs. executing the query string directly.  Now I was using the prepared statements for the logical separation of the query string from the query values and not processing bulk queries where prepared statements are vastly faster on an aggregate scale.  I'd probably suggest against using prepared statements for single-run queries because of the differences in the database engines. 

 

To clarify, and this is only my experience with Postgres and Oracle. 

[zenlord]

very intersting things. I also read that prepared statements are only faster if you need to do the same query more than once (well, more than 100 times or so), so I will need to do some testing before jumping in that one.

 

I was thinking of a db abstraction anyway, but since OO is new to me, I was going to do the conversion in multiple stages - db abstraction being one of the last things on the list. That way I could grow into OO and learn for myself instead of typing stuff that I read in a book. I like it when an app grows slowly and steadily.

 

The way I'm seeing it now is using the PDO-session class to instantiate a persistent connection whenever a session is started (login) and re-use that connection in various transactions until the user logs out. Looks very KISS to me - we'll see how long it stays that way