Roy
Roy

Reputation: 745

Having trouble with associative array structure in PHP

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

Answers (3)

Peter Bailey
Peter Bailey

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

  1. 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)
    
  2. 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)

A few notes on getters

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

  • Methods which are not property getters or setters SHOULD avoid using get and set as a prefix
    • Possible get alternatives: find, fetch, acquire, locate, pull, access
    • Possible set alternatives: push, apply, make, post, mark, attach, assign

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.

A better approach

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

mostlydev
mostlydev

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

gwinh
gwinh

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

Related Questions