...

View Full Version : adding data to a class



cfructose
03-27-2010, 10:55 PM
With a view to making my code for a comments form more reusable, and as an experiment in my struggle to get to grips with OOP, I'm getting a little confused...

I have a config file that contains information about a MySQL database, and the form that'll be used to post comments. Here's a paired-down version - - - just skim-read this code - - - I just put it here so you get the idea of what sort of data there is:



$MySQL['server'] = "localhost";
$MySQL['user'] = "blah";
$MySQL['password'] = "blah";
$MySQL['database'] = "blah";
$MySQL['table'] = "blah";

$MySQL['db_fields'] = array(
'name' => "varchar(100) NOT NULL default ''",
'email' => "varchar(120) NOT NULL default ''",
'website' => "varchar(200) NOT NULL default ''",
'country' => "varchar(40) NOT NULL default ''",
'comment' => "text NOT NULL",
'emailFollowups' => "tinyint(1) NOT NULL default '0'",
'newsletter' => "tinyint(1) NOT NULL default '0'",
'ip' => "varchar(20) NOT NULL default ''",
'admin_comment' => "text NOT NULL default ''",
'is_approved' => "tinyint(1) NOT NULL default '1'",
'date' => "DATETIME DEFAULT NULL"
);

$MySQL['db']['geoIP'] = array('name' => "blah");

$MySQL['db']['guestbook']['form'] = array(
'captcha' => TRUE,
'legend' => "blah",
'hiddenFields' => array(
'formType' => "guestbook"
),
'visibleFields' => array(
'textInput' => array(
'name' => array('label' => "Name", 'required' => TRUE, 'maxlength' => 100),
'email' => array('label' => "Email", 'required' => FALSE, 'maxlength' => 120),
'website' => array('label' => "Website", 'required' => FALSE, 'maxlength' => 200)
),
'comment' => array('label' => "Comment", 'rows' => 15, 'cols' => 55, 'minlength' => 10, 'maxlength' => 5000, 'charCounter' => TRUE),
'checkboxes' => array(
'emailFollowups' => array('label' => "Notify me of follow-up comments via email", 'checked' => FALSE),
'newsletter' => array('label' => "Subscribe to the ".$companyName." newsletter", 'checked' => FALSE)
)
),
'manual' => array(
'is_approved'
),
'autoCalculated' => array(
'geoIP',
'flag',
'date'
)
);

Now, I've made a MySQL class for performing common operations, and all the data from the config file needs to be accessible to the class.

The class goes seomthing like this:


class MySQL{
//Public Properties
public $MySQL;

//Public Methods
function add_MySQL_config($MySQL) {
$this->MySQL = $MySQL;
}

/*
Other methods like:
setDatabase
createTable
getAllWhere
etc
*/
}

OK, 3 questions:

First, is it bad practice to define data outside a class and 'import' it, as I'm doing with:


$MySQLobj = new MySQL();
$MySQLobj->add_MySQL_config($MySQL);

...or is this a standard way of going about it?

Second, when I want to use the MySQL class with a different table or database than the ones I originally sent as arugments to the class (defined in my config array as $MySQL['database'] and $MySQL['table'] ), how should I do it?


Create a new MySQL object and 'import' the new database and table names?
Use the same object but redefine the db and table values of the array $this->MySQL


Third, given that the methods in the MySQL class need to be accessed by another class I've made, such as a class for general form validation,
should I be making the MySQL class an extension of the Form class, thus?


class MySQL extends Form {
}

I'll be amazed if I've expressed myself clearly enough for anyone to actually see what it is that's confusing me, but any comments would be most gratefully received.

cfructose
03-28-2010, 10:53 AM
Having just woken up the following morning and re-read my last post, I already see (I think) the answers to two of my questions. (And I completely understand why there have been no responses!)

The naivety of my questions must have been greeted with eye-rolls, but despite having had a few insights, I still need some guidance. I'm gonna explain my thinking on the off chance that it will help clarify what I was originally asking, which will give anyone hardy enough to read all this something more concrete to respond to.

Regarding my first question:


...is it bad practice to define data outside a class and 'import' it?

Googling "PHP MySQL class" turned up a couple of examples of exactly what I'm trying to do. Both the classes I found do exactly what I was doing, which is reassuring - namely, they 'import' data. I.e. a config file, independant of the class, defines connection information rather than that information being 'hard coded' into the class.

In case anyone who read my initial question is wondering why I would ask something so apparently stupid as "should configurable variables like a database name and password be defined outside a class", it's because I had misconstrued a comment I read by a php guru about 'classes that don't contain any data' being 'bad'. I suppose that got me thinking that the properties of a class need to be originally defined inside that class; that somehow this 'self-containedness' was a fundamental and desirable aspect of the nature of OOP. Naturally, for a class to be reusable (and not just classes - any code), such data would need to be defined outside it, but this blindingly obvious fact passed me by!

Regarding my second question:

The answer must be 'yes'. (Though I'd still like confirmation of this conclusion from someone experienced). That is, I should create a new object for each distinct database.

The whole point of an object is that it is self-contained, that everything in it - properties, methods etc - is a unit. There can then be multiple instances of it, with each instance having different data. But - and this is where I'm still a little confused - if the creation of a new MySQL object were solely for the purpose of changing one bit of 'data' - the database name - then what's the point of creating a new object? All the methods remain the same, and other data inherent to the class remains unchanged, such as my array of MySQL error messages for user feedback (defined inside the class). So, couldn't I just use the same object but with one updated property ($this->db = $newDBname).

Still, it seems less messy to keep things like the database name constant (perhaps even to define it as a constant!), and to have a new object for each database. Yes, writing this confirms to myself that I still don't really know the answer! [yelling out desperately] What should I do and why?

As for my third question about extending the class, I remain at sea.

Here's why:
If some other class I have - let's say a Comments class - contains a method that returns the number of comments that have been posted, then this method will need to use my MySQL class. Consequently the MySQL object will have to be passed as an argument to the Comments object. That's what I've already implemented, and it works fine, but it feels messy because of the restricted scope...

It would be nice to simply have the MySQL object avaiable globally - well, at least, to other classes that depend on it - which is, I suppose, what the php 'extends' is for.

Of course, if I have more that one class that depends on the MySQL class (and I already have three), then I can't very well go about extending the MySQL class to all of them (or can I)?

Do I just lump it, and continue to pass the MySQL object to other classes as and when needed, or have I got it all wrong?

So you see, my third question is really one about the fundamental architecture of class interaction. A little voice at the back of my mind keeps wondering whether I'm writing in unnecessary complexity, or perhaps just thinking procedurally instead of OO-ly.

Dormilich
03-28-2010, 11:26 PM
I would not extend the (say) Form class but the MySQL class, so you can have as many classes using MySQL (IMO, bad name, could be confused with a PHP internal class) as you want.

class Form extends MySQL
on the other hand side, usually you only need some public methods, so you can
- put a MySQL instance as class property

class Form
{
protected $db = NULL;
public function __construct(MySQL $db)
{
$this->db = $db;
}
}
$form = new Form(new MySQL(...));
- use a MySQL Singleton (or Abstract Registry) that acts similar to a Global variable (that’s what I use). restriction is, that you have only one connection at a time.

either way, make the MySQL class implement an Interface (so that all classes can depend on that "MySQL API")

personal recommendation: make your config file an .ini* (read only permission), so every function that requires any setting can read the data (parse_ini_file()) but not change it (e.g. = instead of ==, you’re going bonkers while searching for that)

* - XML would work too

cfructose
03-29-2010, 11:08 AM
bad name, could be confused with a PHP internal class

A-ha! Yes! Right, it's now renamed 'Database'.



class Form extends MySQL

Of course! Thank you for that. I was looking at it back-to-front and worrying about how I could use the database methods in more than one other class, when all the while all I had to do was 'invert the extensions'.

A singleton won't work for me, as I need more than one. :o


either way, make the MySQL class implement an Interface (so that all classes can depend on that "MySQL API")

This helped a lot. I think I understand where to go from here.

Good tips on the .ini and the single equals sign, by the way.
Cheers.

cfructose
03-29-2010, 11:17 AM
Nope, I'm talking nonsense. Of course I only need one connection at a time, so Singleton is the way to go.

I looked at some examples of an abstract class (for me, the Database class) with lots of other subclasses depending on it (Form class etc).

Is that superior is some way to the singleton idea?

Oh, I was confused by this line in your code:


protected $db = NULL;

Why do we want to declare it as NULL?

Dormilich
03-29-2010, 12:13 PM
Nope, I'm talking nonsense. Of course I only need one connection at a time, so Singleton is the way to go.

I looked at some examples of an abstract class (for me, the Database class) with lots of other subclasses depending on it (Form class etc).

Is that superior is some way to the singleton idea?

as always, it depends on the exact circumstances. maybe the obvious advantage is, that you don’t need an object instance to work with it. (Abstract Registry Pattern (http://www.phpbar.de/w/Registry))


Oh, I was confused by this line in your code:
why that?


Why do we want to declare it as NULL?
sensible default value.

cfructose
03-30-2010, 06:07 PM
@Dormilich

Thanks for all your help do far.
I've now successfully put together an abstract Database class with MySQL config stuff in a .ini

Having done this, I've got a few questions...


personal recommendation: make your config file an .ini (read only permission), so every function that requires any setting can read the data (parse_ini_file()) but not change it (e.g. = instead of ==, you’re going bonkers while searching for that)

1) Are .ini files 'read only' by definition, or are you suggesting that I configure that on the server?

2) When you mention the single equals sign, do you mean in a line like...?
$database_ini = parse_ini_file($path);
If you don't mean that equals sign, then I can't think what you're referring to, nor why the comparison operator "==" could be relevant. I've clearly missed your point! It sounds like a tip too good to miss, and I'm sure I don't want to go "bonkers" looking for something (especially when I don't know what I'm looking for!).

My code as it stands goes something like this... Please give it the once-over, and tell me if it all looks like 'good practice':


;database.class.php
;<?php die("Don't try to access this page directly!"); ?>
[database]
host = localhost
user = "user_name"
password = "password_including_non-alphanumeric_chars_hence_double_quotes"

;More variables here

;remember the line feed at the end of the .ini.php in order for parse_ini_file to work in PHP > 5.3

and then:


//database.class.php
$database_ini = parse_ini_file(CONSTANT_PATH_TO_PRIVATE_FOLDER_ABOVE_PUBLIC_HTML."database.ini.php", TRUE);
define('DB_SERVER', $database_ini['database']['host']);
define('DB_USER', $database_ini['database']['user']);
define('DB_PASSWORD', $database_ini['database']['password']);

abstract class Database{
protected $host, $user, $pw, $db;

public function __construct($db = NULL, $host='', $user='', $pw='') {
$this->host = (!empty($host)) ? $host : DB_SERVER;
$this->user = (!empty($user)) ? $user : DB_USER;
$this->pw = (!empty($pw)) ? $pw : DB_PASSWORD;
$this->db = $db;
}

//etc
}

3) My "$database_ini = parse_ini_file(...)" is currently on the same page as the class, but outside the class {}. Is that where people usually put it in order for the .ini to be included automatically, or is there some better way?

4) I reckon 'protected' is probably best for these properties. Or would private be better?

If I'm still veering off in the wrong direction, I look forward to being shouted at, but if all this looks on track then please do let me know... I'm still feeling very insecure and out of my depth with OOP that's so new to me.

Thanks for your time.

Dormilich
03-31-2010, 06:57 AM
1) .ini files are not read-only by default. I suggested read-only because you certainly don’t want such data overwritten in the course of a script. by using parse_ini_file() it is not necessary to write protect the file, but you still can change it with fwrite() or file_put_contents().

2) I mentioned that when you wrote about global variables.

// imagine the following
if ($DB['pass'] == "password2")
// vs.
if ($DB['pass'] = "password2")
simple error but devastating result

3) I’d do the reading of the .ini file inside the class, this way the sensitive data don’t leave the class, they stay protected/private, while currently you have them globally available.

4) I’d use private, unless a descendent class requires those values.

cfructose
03-31-2010, 07:59 AM
Of course! the parse ini can be inside the class! D'oh!

That little '=' v. '==' used to get me all the time, but never with "devastating" results. I can see the danger though.

You've been great. I couldn't imagine more satisfactory answers. Thanks so much.

cfructose
03-31-2010, 08:14 AM
One more thing (just one more, I promise!) ;)

If I'm going to use the parse_ini_file inside the class, then should it be in the __construct, thus?



abstract class Database{
private $host, $user, $pw, $db, $db_ini;

public function __construct($db = NULL, $host='', $user='', $pw='') {
$this->db_ini = parse_ini_file(PRIVATE."database.ini.php", TRUE);

$this->host = (!empty($host)) ? $host : $this->db_ini['database']['host'];
$this->user = (!empty($user)) ? $user : $this->db_ini['database']['user'];
$this->pw = (!empty($pw)) ? $pw : $this->db_ini['database']['password'];
$this->db = $db;
}

//...
}

Dormilich
03-31-2010, 08:47 AM
If I'm going to use the parse_ini_file inside the class, then should it be in the __construct, thus?

you can do it this way, but you’re not required to. you just have to make sure you do it, before you need it. (if that code would be more complex, you can do that in its own method and call this method in __construct())

cfructose
03-31-2010, 09:37 AM
Got it!

Again, thanks.

Now, I know I promised that that would be the last question on this matter, but as my new little problem is so directly relevant to the code already posted in this thread, I figure it'd silly to start a new one...

Bear in mind that this is the first time I've ever tried to extend a class or use __construct, so I apologise if this is a silly question.

When echoing $this->db from the Database class, the value has been successfully set from the .ini. Great.

But the following echoes nothing, as if $db is not set.


class Form extends Database{
public $db;

public function __construct() {
parent::__construct();

//initialise other stuff here, hence needing the parent::construct()

echo 'db = '.$this->db;
}
//...
}

$FormObj = new Form('database_name');


Is it not the case that $db has been inherited from the Database class, and that $this->db should now be set? ($this refers to the current Form object, right?)

And given that Form extends Database, is it still necessary to add:


public $db;

to the Form class?

cfructose
03-31-2010, 09:43 AM
OK, I get it...

This:


public function __construct() {
parent::__construct();
}

should have been:


public function __construct($db) {
parent::__construct($db);
}

I'll now trundle off with my tail between my legs.

Dormilich
03-31-2010, 10:10 AM
still something to mention.

if you define private $db, you implicate that $db must not be used by descendent classes. (see here (http://www.php.net/manual/en/language.oop5.visibility.php))

if you want to access $db in descendent classes, rather use protected $db, then there is also no need to redefine $db in the new class.

cfructose
03-31-2010, 10:58 AM
I just worked that out before reading your comment.

Still great to have such things confirmed though... You know, you've found a solution, but there's a nagging feeling that it might not be the best way.
So it's great to read from you now that what I just did was correct.

Cheers.



EZ Archive Ads Plugin for vBulletin Copyright 2006 Computer Help Forum