Reputation: 745
Not quite sure how to phrase this question but I'm having trouble with the structure of an associate array in a class I'm writing. The relevant parts of the code goes something like this:
class User {
public $userData;
function getUserData() {
$stmt = $this->conn->prepare('SELECT * FROM user_table WHERE id = :id');
$stmt->execute(array('id' => $this->ID));
$row = $stmt->fetchAll();
foreach($row AS $key => $val) {
$this->userData[$key] = $val;
}
return $this->userData;
}
I want to be able use this to set class properties so I can access them like so:
$this->userData['username'];
for example, but this isn't working for me. Instead I have to access them like this for it to work:
$this->userData[0]['username'];
I'm having to add the array index on to access any of the data, which I don't want to be doing but I don't know how to fix this.
Upvotes: 0
Views: 46
Reputation: 105868
This has to do with how PDOStatement::fetchAll() returns row data.
Even though you're only selecting a single row, it will still be returned with a row index (of zero), i.e., if you did this
print_r($row);
You'd get something like this
Array
(
[0] => Array
(
[username] => Roy
[0] => Roy
)
)
Note that by default, fetchAll() returns each column twice in the data - once by column name and once by column index
The fact that you named your variable $row
is probably part of what's tripping you up - semantically that should be $rows
.
To fix this, you have a couple few choices
Directly access the first row in the result
$rows = $stmt->fetchAll(PDO::FETCH_ASSOC); // Notice the addtion of the
foreach($rows[0] AS $key => $val)
Use PDOStatement::fetch() instead
$row = $stmt->fetch(PDO::FETCH_ASSOC); // Notice the addtion of the
foreach($row AS $key => $val)
But in both cases, the for loop is unnecessary. You could simply do
$this->userData = $stmt->fetch(PDO::FETCH_ASSOC);
Lastly, I must question why User::$userData
is public but you've also written a getter for it (which, academically speaking, isn't even a getter - it's a fetcher)
It's not uncommon for people to use get as the verb of choice when writing methods that return something. However, there is an equally broad school of thought that getters should be restricted to returning class members only. Where I work, we have these lines in our coding standard regarding the naming of methods
get
and set
as a prefix
This means developers are allowed to write get*()
methods that don't just return a class property, but they're encouraged not to.
So when I say "fetcher", that is a colloquial term, and not really part of the common software engineering vernacular.
Here's an example where we've applied some separation of concerns rigor. The "getting" and "fetching" of the user data each have their own implementation i.e., their own method.
class User
{
private $userData = null;
/**
* Lazy-load-style getter for $userData
*
* @return array
*/
public function getUserData()
{
if (null === $this->userData)
{
$this->userData = $this->fetchUserData();
}
return $this->userData;
}
/**
* Fetch the user's data from the database
*
* @return array Associative array of user data where the keys
* are column names
*/
private function fetchUserData()
{
$stmt = $this->conn->prepare('SELECT * FROM user_table WHERE id = :id');
$stmt->execute(array('id' => $this->ID));
return $stmt->fetch(PDO::FETCH_ASSOC);
}
}
Although I'd suggest adding a guard check to User::fetchUserData()
in case PDO::fetch()
returns FALSE. Perhaps something like
$user = $stmt->fetch(PDO::FETCH_ASSOC);
if (false === $user)
{
throw new Exception("User with id ($this->ID) not found");
}
Upvotes: 2
Reputation: 723
The reason is because in your example the $key to each row is the row index, so your assignment statement is, for row 0:
$this->userData[0] = $val;
What you want to do instead is to assign the values to the array using the value of two fields (I'm guessing):
$this->userData[$val['field1']] = $val['field2'];
Or,
$this->userData['username'] = $val['username'];
However, the real problem is that, I think, you're trying to get a single row and pulling an array of results instead -- and the inadvertently iterating over rows, instead of what you think are fields.
If you're looking for a single user record, then you need to use associative single fetch, instead of fetchAll:
class User {
public $userData;
function getUserData() {
$stmt = $this->conn->prepare('SELECT * FROM user_table WHERE id = :id');
$stmt->execute(array('id' => $this->ID));
$this->userData = $stmt->fetch(PDO::FETCH_ASSOC);
return $this->userData;
}
Upvotes: 1
Reputation: 343
The problem is your $stmt->fetchAll()
, in your query it does imply that you are trying to get a single row. You need to replace it with $stmt->fetch()
Upvotes: 1