Hi,
I'd agree with the point about using JOIN in your query - this is the kind of thing that will matter when you're ready to start developing high-traffic applications. Including a JOIN in your query can obviously make things more critical because if the value you're JOINing doesn't exist for some reason then the query will fail.
I'll also just add that it's a bit redundant to assign values from your query into variables if you can access them directly from within the DB object and you're not going to change them in any way - compare
Code: Select all
$db = new MySQL($config['mysql_host'],$config['mysql_user'],$config['mysql_password'],$config['mysql_db']);
$sql = $db->query("SELECT * FROM users WHERE username='$username'");
$result = $db->fetcharray($sql);
$firstName = $result['first_name'];
$lastName = $result['last_name'];
$userid = $result['id'];
?>
<div class="left">
<h3><span class="username"><?php echo $firstName." ".$lastName;?></span> Lists</h3>
</div>
with
Code: Select all
$db = new MySQL($config['mysql_host'],$config['mysql_user'],$config['mysql_password'],$config['mysql_db']);
$sql = $db->query("SELECT first_name,last_name,id AS user_id FROM users WHERE username='$username'");
$result = $db->fetcharray($sql);
?>
<div class="left">
<h3><span class="username"><?php echo $result['first_name']." ".$result['last_name'];?></span> Lists</h3>
</div>
What I do in my DB handler is include a function that lets me change the value retrieved by the result if (for example) I need to format the name in some way or pad an ID:
Code: Select all
class DB {
public $number_of_results;
public $result_array = array();
public $row;
function __construct($sql,$conn) {
$this->result = mysql_query($sql,$conn) or die(mysql_error());
$this->number_of_results = mysql_num_rows($this->result);
for ($i = 0; $this->result_array[$i] = mysql_fetch_assoc($this->result); $i++);
array_pop($this->result_array);
$this->row = 0;
}
function set_val($key,$value) {
$this->result_array[$this->row][$key] = $value;
}
function val($key) {
return $this->result_array[$this->row][$key];
}
function reset_row() {
$this->row = 0;
}
function get_row() {
return $this->result_array[$this->row];
}
}
// Example usage
$db = new DB("SELECT first_name, surname FROM tbl_users WHERE user_id = '$user_id' LIMIT 1",$this->get_conn());
if ($db->number_of_results == 1) {
echo $db->val('first_name');
}
Please note that this is not exactly AAA code, but it does work
On a final point, specifying the values individually rather than just using SELECT * is a good "aide-memoire" in your script to remind you of what data is supposed to be being retrieved, plus it's good design practice to only retrieve the values your script needs - your script will inadvertently retrieve the user's password and other sensitive data if you just use the wildcard.
HTH,
Mecha Godzilla