Re: OOP, classes and databases [message #169414 is a reply to message #169408] |
Fri, 10 September 2010 19:48 |
Thomas Mlynarczyk
Messages: 131 Registered: September 2010
Karma:
|
Senior Member |
|
|
Mattias Campe schrieb:
> - PDO is better than mysql_query
Yes. And use prepared statements.
> - __construct is the way of making constructors
Yes.
> - __get and __set is the way for getting and setting properties
No. Usually properties are private and you write getFoo() and setFoo(
$value ) methods to retrieve and set their values in a controlled way
(allowing to validate the value to be set, for example). There may be
circumstances where it is okay to allow anyone unrestricted access to a
property. In this case, you can make the property public and don't need
__get/__set. If you want to control access to a property, but still
allow the simpler syntax $object->foo = 42, then you can use
__get/__set. Also, if you want to allow public read-only access to a
private property, you can use __get to achieve that.
> class Person {
> private $name;
> private $address;
>
> public function __construct ($id,$db) {
> $query = ... the select query ...
> $row = $db->query($query);
That last line above is not needed as you re-do the same query in the
foreach loop below.
> foreach ($db->query($query) as $row) {
> $this->name = $row['name'];
> $this->address = $row['address'];
And anyway, that query should return at most one result (it is about one
specific person, isn't it?), so there's no need for the loop. You could
just do:
public function __construct( $id, PDO $db )
{
// Create a prepared statement (the ? is a placeholder)
$stmt = $db->prepare( 'SELECT * FROM persons WHERE Id = ?' );
// Execute that statement replacing the ? with the given $id
$stmt->execute( array( $id );
// Retrieve the one expected row as an object
$row = $stmt->fetchObject();
if ( $row ) // If the query succeeded
{
$this->name = $row->name;
$this->address = $row->address;
}
else // If the record could not be found
{
echo 'There is no person with this id in the database.';
}
}
Note: This code is untested and may even contain syntax errors or other
typos. It is just meant for illustration.
> public function __get ($var) {
> switch ($var) {
> case 'Name':
You mean lowercase 'name' here.
> return $this->name;
> break;
> case 'address':
> return $this->address;
> break;
> default:
> break;
This could be simplified like this:
public function __get( $var )
{
return isset( $this->$var ) ? $this->$var : null;
}
Note: This would allow public read-only access to *all* properties. To
restrict access to just certain properties, you could use this:
public function __get( $var )
{
return in_array( $var, array( 'name', 'address' ) )
? $this->$var
: null;
}
> But I have the feeling that this isn't a 'best practice':
> - I can't use this class without database
You could pass it an object which has the same methods as the PDO
object, but retrieves data in a completely different way (like reading
it from a text file or from an array). What you did is called dependency
injection and is actually indeed a 'best practise'. On the other hand --
since your Person class seems to be just a simple data container, you
might as well do it the other way round and tell PDO to give you back
the data in form of a Person object:
$stmt->fetchObject( 'Person' );
But then you would need to make the properties public, so PDO can access
them to put the values there (and remove all arguments from the
constructor as they would not be needed). In this scenario, you would
not pass the PDO object to the Person constructor, but pass the Person
class to the PDO statement instead.
> - should I put the select query and getting the data in a seperate
> function, outside the constructor?
You could do it the way I suggested above. In any case, the name and
address properties must be set when the Person object is created. You
must not allow a Person object to exist without having its name and
address set!
Greetings,
Thomas
--
Ce n'est pas parce qu'ils sont nombreux à avoir tort qu'ils ont raison!
(Coluche)
|
|
|