PHP: Proper way of using a PDO database connection in a class

Question

Trying to organize all my code into classes, and I can't get the database queries to work inside a class. I tested it without the class wrapper, and it worked fine. Inside the class = no dice. What about my classes is messing this up?

EDIT: The problem is that it won't query the database and return a result. Any result.

class ac
  {
  public function dbConnect() 
    {
    global $dbcon;

    $dbInfo['server'] = "localhost";
    $dbInfo['database'] = "sn";
    $dbInfo['username'] = "sn";
    $dbInfo['password'] = "password"; 

    $con = "mysql:host=" . $dbInfo['server'] . "; dbname=" . $dbInfo['database'];
    $dbcon = new PDO($con, $dbInfo['username'], $dbInfo['password']);
    $dbcon->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
    $error = $dbcon->errorInfo();

    if($error[0] != "") 
      {
      print "<p>DATABASE CONNECTION ERROR:</p>";
      print_r($error);
      }
    }


  public function authentication()
    {
    global $dbcon;

    $plain_username = $_POST['username'];
    $md5_password = md5($_POST['password']);

    $ac = new ac();
    if (is_int($ac->check_credentials($plain_username, $md5_password)))
      {
      ?>
      <p>Welcome!</p> <!--go to account manager here-->
      <?php
      }
    else
      {
      ?>
      <p>Not a valid username and/or password. Please try again.</p>
      <?php
      unset($_POST['username']);
      unset($_POST['password']);
      $ui = new ui();
      $ui->start();
      }
    }



  private function check_credentials($plain_username, $md5_password)
    {
    global $dbcon;

    $userid = $dbcon->prepare('SELECT id FROM users WHERE username = :username AND password = :password LIMIT 1');
    $userid->bindParam(':username', $plain_username);
    $userid->bindParam(':password', $md5_password);
    $userid->execute();

    print_r($dbcon->errorInfo());

    $id = $userid->fetch();
    Return $id;
    }
  }

And if it's any help, here's the class that's calling it:

require_once("ac/acclass.php");
$ac = new ac();
$ac->dbconnect();
class ui
  {
  public function start()
    {
    if ((!isset($_POST['username'])) && (!isset($_POST['password'])))
      {
      $ui = new ui();
      $ui->loginform();
      }
    else
      {
      $ac = new ac();
      $ac->authentication();
      }
    }

  private function loginform()
    {
    ?>
    <form id="userlogin" action="<?php echo $_SERVER['PHP_SELF']; ?>" method="post">
     User:<input type="text" name="username"/><br/>
     Password:<input type="password" name="password"/><br/>
     <input type="submit" value="submit"/>
    </form>
    <?php
    }
  }

do you mean the query fails (i.e. produces an error) or that the query succeeds but the result set is empty? — outis
– outis, Commented Mar 25, 2010 at 20:18
Not sure. I don't get any error, but nothing is returned. I've gone so far as to place the username and password right in the query, and I still get no matches — JoeCortopassi
– JoeCortopassi, Commented Mar 25, 2010 at 20:24
if there's an error, the statement should throw a PDOException, and set PDOStatement::errorCode and PDOStatement::errorInfo. If there's simply an empty result set (meaning the query succeeded with no matches), PDOStatement::rowCount should return 0 with MySQL (however, see the doc php.net/PDOStatement.rowCount) and the PDOStatement::fetch* methods will return False. — outis
– outis, Commented Mar 25, 2010 at 20:55

outis · Accepted Answer · 2010-03-25 21:11:35Z

The default for PDOStatement::fetch is to return a row as an array of fields, indexed by column name and number (mode PDO::FETCH_BOTH). This means ac::check_credentials returns an array, but ac::authentication checks for an integer. Also, field values are strings, so is_int will fail unless you explicitly convert the result field to an integer. Try PDOStatement::fetchColumn() and is_numeric.

    public function authentication() {
        ...
        if (is_numeric($this->check_credentials($plain_username, $md5_password))) {
        ...
    }

    private function check_credentials($plain_username, $md5_password) {
        return $userid->fetchColumn();
    }

As an alternative to is_numeric, check that the result of the credential check isn't identical to False.

    public function authentication() {
        ...
        if (False !== $this->check_credentials($plain_username, $md5_password)) {
        ...
    }

Some Stylistic Points

As Yaggo points out, ui::start and ac::dbConnect should be static methods. ac::authentication doesn't need to create a new ac; since it's not a static method, it can access the current object via the $this special variable (as is done above). $dbcon should be made a static property of ac so you don't pollute the global namespace. Class names should use UpperCamelCase, and should be more descriptive.

Classes should have a single, well-defined purpose and not stray from that. ac has many purposes: manage a DB connection, handle authentication and display results of authentication. Consider designing a set of classes for a Data Access Layer, hiding the database from everything else. Also consider separating the domain logic (authentication &c) from display. This is part of the pattern known as the Model View Controller architecture. This doesn't need to happen all at once; you can slowly refactor the code.

+1 + FYI: UpperCamelCase has a "real" name too: PascalCase. which is not camelCase

jholster · Accepted Answer · 2010-03-25 20:09:15Z

1

Won't solve your problem, but if you are using the classes just as namespaces to group similar functions together, consider using static methods to avoid the overhead of creating an instance.

class Foo {

    static function bar() {

    }

}

Foo::bar()

answered Mar 25, 2010 at 20:09

jholster

5,1361 gold badge29 silver badges21 bronze badges

Collectives™ on Stack Overflow

PHP: Proper way of using a PDO database connection in a class

2 Answers 2

Some Stylistic Points

1 Comment

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Some Stylistic Points

1 Comment

Comments

Your Answer

Sign up or log in

Post as a guest

Related