variables in classes - correct approach

Question

This code will output the same. My question is, what is the correct way to do this. The first approach or the second? Or there is any better way? I don't see any advantage of one Class over the other.

<?php class Client{ var $id; var $email; function __construct($id){ $this->id=$id; } public function get_email($db){ $sql = $db -> prepare(" SELECT email FROM users WHERE id = ? "); $sql -> bind_param('i', $this->id); $sql->execute(); $sql->bind_result($email); if ($sql -> fetch()) { return $this->email=$email; } else return false; } } class Client_{ public function get_email($db, $id){ $sql = $db -> prepare(" SELECT email FROM users WHERE id = ?"); $sql -> bind_param('i', $id); $sql->execute(); $sql->bind_result($email); if ($sql -> fetch()) { return $email; } else return false; } } ?>

index.php

<?php $Client = new Client(1); $a = $Client -> get_email($db); print_r($a); $Client_ = new Client_(); $b = $Client_ -> get_email($db, 1); print_r($b); ?>

Why are you passing the $id as a string? It can just be an int, right? — Jon Egeland
– Jon Egeland, Commented Apr 23, 2012 at 1:59
I think the best would be when you pass the $db in the contructor and then execute function get_email method every time you want some data. — hamczu
– hamczu, Commented Apr 23, 2012 at 2:04
What's "more correct" depends on what you want to express or model with your class. — deceze
– deceze ♦, Commented Apr 23, 2012 at 2:04
Should an instance of the class represent a specific user, and calling get_email gets that user's email? Or does the class rather just model the database table and represents an interface to query that specific table, not tied to a specific user instance? — deceze
– deceze ♦, Commented Apr 23, 2012 at 2:16

Sampo Sarrala · Accepted Answer · 2012-04-23 02:15:10Z

In second approach it does not make sense to instantiate class, as there is nothing to be stored for future use. So it would be better to use "static class" if varying data is stored somewhere else:

class Client_{ static public function get_email($db, $id){ $sql = $db -> prepare(" SELECT email FROM users WHERE id = ?"); $sql -> bind_param('i', $id); $sql->execute(); $sql->bind_result($email); if ($sql -> fetch()) { return $email; } else return false; } } // And use it static way without instantiating first: Client_::get_email( $arg1, $arg2 );

If i shoul make decision between those two I'm going to take first one.

I dont know how you are going to use either one of those classes but still for me it makes more sense to store $db and supply $id from outside and make $email local:

class Client{ var $db; function __construct($db){ $this->db=$db; } public function get_email($id){ $sql = $this->db -> prepare(" SELECT email FROM users WHERE id = ? "); $sql -> bind_param('i', $id); $sql->execute(); $sql->bind_result($email); if ($sql -> fetch()) { return $email; } else return false; } }

Also changed this line: return $this->email=$email;, maybe I got it wrong but I think that it just doesn't make sense.

Hubro · Accepted Answer · 2012-04-23 02:02:38Z

I'd say the first one is more "correct" in this scenario since the class name indicates that the class is a model for a database table, meaning that most or all changes to the table should be abstracted through the class. Often the instantiation of the model class will represent one row of the database table, so it makes sense that the id is a member of the class.

I would also pass the database connection to the model through the constructor, or somehow have it globally available.

^ Just my 2 cents

tereško · Accepted Answer · 2012-04-25 07:06:16Z

Please stop using var to define class variables in php. It is not 4.x anymore. Now where have public, private and protected.

That said, the instance of Client require both connection and identificator:

class Client { protected $connection; protected $id; protected $email = null; public function __construct($connection, $id) { $this->connection = $connection; $this->id = $id } public function getEmail() { if ( $this->email === null ) { $query = 'SELECT email FROM users WHERE id = ?'; $statement = $this->connection->prepare( $query ); $statement->bind_param('i', $this->id); if ( $statement->execute() ) { $statement->bind_result($this->email); } } return $this->email; } }

P.S. i would actually prefer PDO instead of MySQLi for connection API. If for not other reason, then just because it has more flexibility in error handling.

Collectives™ on Stack Overflow

variables in classes - correct approach

3 Answers 3

Comments

Comments

Comments

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Comments

Comments

Comments

Related