PHP Designing Interfaces in PHP Introducing Interfaces Implement MySQL RepositoryInterface

Julie Dowler
Julie Dowler
7,849 Points

I'm stumped on this one...

I got through the first part of the challenge, which was to create the "all" function. But when I had to create the "find" function and bind parameters to a prepared statement, it says it's no longer passing. Here's what the challenge asks me to do:

Challenge Task 3 of 3

Define the "find" method that accepts the required parameters.

Write a prepared statement that selects only the items that match the table, value and field passed to the method. Make sure you bind the parameters to the statement.

Return an array of query results.
sqlRepository.php
<?php
class sqlRepository extends PDO implements RepositoryInterface

  {

    public function all($databaseTable)
    {
    $db = new PDO("sqlite:".__DIR__."/database.db");  
      try {
      $results = $db->query('select * from '.$databaseTable);
      }  catch (Exception $e) {
        echo $e->getMessage();
        die();
      }

       $values = $results->fetchAll(PDO::FETCH_OBJ);
      return $values;
    }

    public function find($databaseTable, $value, $field = 'id');
    {
    $db = new PDO("sqlite:".__DIR__."/database.db");
      try {
      $results = $db->prepare('select * from :databaseTable where :field=:value');
      $results->bindParam(':databaseTable', $databaseTable, PDO::PARAM_STR);
      $results->bindParam(':field', $field, PDO::PARAM_STR);
      $results->bindParam(':value', $value, PDO::PARAM_STR);
       $results->execute();
      }  catch (Exception $e) {
        echo $e->getMessage();
        die();
      }

       $values = $results->fetchAll(PDO::FETCH_OBJ);
      return $values;
    } 


}

7 Answers

Simon Coates
Simon Coates
28,648 Points

Once, i fixed that issue, i noticed another issue. So I messed around with your code until i got something it accepts. I haven't done the videos and it's been an age since i did PHP, so it's probably deeply flawed. However at the time i tested it, it seemed to accept:

<?php
class sqlRepository extends PDO implements RepositoryInterface
{
    public function all($databaseTable)
    {
        //$db = new PDO("sqlite:".__DIR__."/database.db");  
        try {
            $results = $this->query('select * from '.$databaseTable);
        }  catch (Exception $e) {
            echo $e->getMessage();
            die();
        }

        $values = $results->fetchAll(PDO::FETCH_OBJ);
        return $values;
    }

    public function find($databaseTable, $value, $field = 'id') // *
    {
      $query = "SELECT * FROM $databaseTable WHERE $field = :value";
      //see https://stackoverflow.com/questions/182287/can-php-pdo-statements-accept-the-table-or-column-name-as-parameter
      try {
        $results = $this->prepare($query);
        $results->bindParam(':value', $value, PDO::PARAM_STR);
        $results->execute();
      }  catch (Exception $e) {
        echo $e->getMessage();
        die();
      }
       $values = $results->fetchAll(PDO::FETCH_OBJ);

      return $values;
    }
  }

I don't know if Alena Holligan covered the restrictions on binding. However, it makes sense that you don't bind in tables and fields. See the link in the code comment.

The prepare statement in your original code seemed to return a false, rather than a PDOStatement, hence the subsequent method call failed (you might care to demo this). As mentioned, the issue seemed to be that it rejected the query. To get around this, i used the field and table values directly in the query. I don't know if this represents a problem for sql injection.

Alena Holligan
Alena Holligan
Treehouse Teacher

Good call Simon Coates ! yes, you can't bind the table and field NAMES because it is escaping/securing the value! I updated the code challenge to help people through this scenario :)

Simon Coates
Simon Coates
28,648 Points

the initial problem is:

public function find($databaseTable, $value, $field = 'id');

It's the ;

Julie Dowler
Julie Dowler
7,849 Points

No, I'm pretty sure she didn't cover the restrictions on binding. Not in this video series, anyway. I did it your way and it worked. It was also important to use double quotes around the query instead of single. Here's what finally passed:

<?php
public function find($databaseTable, $value, $field = 'id')
{
    $db = new PDO("sqlite:".__DIR__."/database.db");
    try{
    $query = "select * from $databaseTable where $field = :value";
    $results = $db->prepare($query);
    $results->bindParam(":value",$value,PDO::PARAM_STR);
    $results->execute();
    }  catch (Exception $e)  {
    echo $e->getMessage();
    die();
    }
    $values = $results->fetchAll(PDO::FETCH_OBJ);
     return $values;
    $db->connection = null;
 }
Simon Coates
Simon Coates
28,648 Points

I had noticed you'd created a separate PDO object. Given that the current object extends (IS A) PDO, i had wondered if it made more sense to try and call on the current object. I was a little confused over whether it was necessary to provide a constructor (Alena Holligan seemed to direct another user to documentation on this here ), but it seemed to work - the methods seem to be called on a constructed PDO (sqlRepository) object, (which might already have access to a dsn string).

Alexandru Palita
Alexandru Palita
14,245 Points

This worked for me... No need for declaring $db ... it will be declared probably in Collection class later...

Check out https://teamtreehouse.com/library/extending-objectoriented-php Connection class on WorkSpace

<?php
class sqlRepository extends PDO implements RepositoryInterface
{
  public function all($databaseTable) 
   { 
      $sql = "SELECT * FROM $databaseTable";
      $results = $this->prepare($sql);
      $results->execute();
      return $results->fetchAll(PDO::FETCH_OBJ);
    }

  public function find($databaseTable, $value, $field = 'id')
    {
      $sql = "SELECT * FROM $databaseTable WHERE $field = ?";
      $results = $this->prepare($sql);
      $results->bindParam(1, $value);
      $results->execute();
      return $results->fetchAll(PDO::FETCH_OBJ);
    } 

}
Amber Stevens
Amber Stevens
Treehouse Staff

That is the solution that also worked for me. My code was almost identical to yours although I used $entity instead of $databaseTable, but it was the same thing and I finally passed.

Julie Dowler
Julie Dowler
7,849 Points

It seems like opening and closing the database in each method is probably not the best way of doing it, but I don't know any other way.

Now, after changing the challenge question, Alena Holligan has re-ordered the videos in this course. I don't think they're in the right order any more. The first video seems to refer to things that have already been talked about.

Alena Holligan
Alena Holligan
Treehouse Teacher

Because you are extending the PDO class, you will already have the database connection. You don't need to new object line at all. Just use the $this keyword

$statement = $this->prepare($query);

What things seem to be out of order?

Julie Dowler
Julie Dowler
7,849 Points

I'm pretty sure that I had to open a database connection to get it to work. Anyway, the whole challenge has disappeared now and the course has been re-done. Are you sure the videos are in the right order?

Alena Holligan
Alena Holligan
Treehouse Teacher

I don't know what you're referring to when you say that the challenge has disappeared and the course has been redone. https://teamtreehouse.com/library/designing-interfaces-in-php

The challenge is at the very end of stage 1 "Introducing Interfaces"

Julie Dowler
Julie Dowler
7,849 Points

Nevermind. It's not really out of order. I was just confused.