OOP problem: Call to a member function....

[naike]

So heres my code:

<?php
include_once $_SERVER['DOCUMENT_ROOT'] . "/includes/class_database.php";
?>

<?php

class User {    

public static function find_all() {
$result = $database->db_query("SELECT * FROM users");
return $result;    
}

public static function find_by_id($id=0) {
$result = $database->db_query("SELECT * FROM users WHERE id={$id}"); // Line 26!
$final = mysqli_fetch_array($result);
return $final;    
}

}
?>

<?php
include_once $_SERVER['DOCUMENT_ROOT'] . "/includes/values.php";
include_once $_SERVER['DOCUMENT_ROOT'] . "/includes/class_user.php";
?>

<?php
class MySqlDatabase {
    private $connection;

function __construct() {
$this->db_connect();
$database = new MySqlDatabase(); // ------I'm instantiating $database!------
}

public function db_connect() {
$this->connection = mysqli_connect(DB_SERVER, DB_USERNAME, DB_PASSWORD, DB_NAME); 

    if (!$this->connection) {
        die("Database connection failed: " . mysqli_error($this->connection));
        }

}

public function db_query($sql) {
$result = mysqli_query($this-connection, $sql);
    if (!$result) {
        die("Database query failed: " . mysqli_error($this->connection));
    }
return $result;

}

}

Here's the code I'm trying to execute:

<?php
include_once $_SERVER['DOCUMENT_ROOT'] . "/includes/values.php";
include_once $_SERVER['DOCUMENT_ROOT'] . "/includes/functions.php";
include_once $_SERVER['DOCUMENT_ROOT'] . "/includes/class_database.php";
include_once $_SERVER['DOCUMENT_ROOT'] . "/includes/class_user.php";
?>

<?php

$found = User::find_by_id(1);
echo $found;

?>

Error received:
Fatal error: Call to a member function db_query() on a non-object in ..\includes\class_user.php on line 26
As far as I know this would happen if I wouldn't instantiate $database = new MySqlDatabase(); but I do it in my constructor, and I include the class_database.php file in my class_user.php file.

[NogDog]

The immediate problem is that $database is undefined within the scope of the methods in your User class. If you want to use those methods statically, then you'll probably have to pass the database object to each as an additional argument. Personally, I'd prefer to not use them statically and just pass it to the constructor:

class User
{
   private $database;
   public function __construct($database)
   {
      $this->database = $database;
   }
   public function find_all()
   {
      // use $this->database:
      $result = $this->database->db_query("SELECT * FROM users");
      return $result;
   }
   public function find_by_id($id = 0)
   {
      $result = $this->database->db_query("SELECT * FROM users WHERE id={$id}");
      $final = mysqli_fetch_array($result);
      return $final;
   }
}

Usage:

$database = new MySqlDatabase();
$user = new User($database);
$found = $user->find_by_id(1);

Something else to consider is that MySqlDatabase could simply extend the built-in MySQLi class, then have your class call the MySQLi constructor:

class MySqlDatabase extends mysqli
{
   public function __construct()
   {
      parent::__construct(DB_SERVER, DB_USERNAME, DB_PASSWORD, DB_NAME);
      if($this->connect_error) {
         throw new Exception($this->connect_error);
      }
   }

   // your custom functions go here . . .

}

[naike]

NogDog;10965869 wrote:

The immediate problem is that $database is undefined within the scope of the methods in your User class. If you want to use those methods statically, then you'll probably have to pass the database object to each as an additional argument. Personally, I'd prefer to not use them statically and just pass it to the constructor:

class User
{
   private $database;
   public function __construct($database)
   {
      $this->database = $database;
   }
   public function find_all()
   {
      // use $this->database:
      $result = $this->database->db_query("SELECT * FROM users");
      return $result;
   }
   public function find_by_id($id = 0)
   {
      $result = $this->database->db_query("SELECT * FROM users WHERE id={$id}");
      $final = mysqli_fetch_array($result);
      return $final;
   }
}

Usage:

$database = new MySqlDatabase();
$user = new User($database);
$found = $user->find_by_id(1);

Something else to consider is that MySqlDatabase could simply extend the built-in MySQLi class, then have your class call the MySQLi constructor:

class MySqlDatabase extends mysqli
{
   public function __construct()
   {
      parent::__construct(DB_SERVER, DB_USERNAME, DB_PASSWORD, DB_NAME);
      if($this->connect_error) {
         throw new Exception($this->connect_error);
      }
   }

   // your custom functions go here . . .

}

Can you explain this to a dummy, I'm pretty new at php.
I just tried to add a global variable $database to the User methods, but I still get the same error.

I don't quite understand your code, can you explain it?
In a read simple way.

[NogDog]

See if this makes sense to you. I've done a fair bit of rewriting, but tried to keep your original intent.

MySqlDatabase class:

/**
 * Database class using MySQLi
 */
class MySqlDatabase extends MySQLi
{
   // no need for a connection class variable if we do it this way //

   /**
    * Constructor
    * Requires that the constants DB_SERVER, DB_USERNAME, DB_PASSWORD, and
    * DB_NAME be defined already, else throws exception
    * @return void
    */
   function __construct()
   {
      if(!defined('DB_SERVER') or !defined('DB_USERNAME') or
          !defined('DB_PASSWORD') or !defined('DB_NAME')) {
         throw new Exception('Missing one or more DB constant');
      }
      // call the MySQLi __construct() method using the constants:
      parent::__construct(DB_SERVER, DB_USERNAME, DB_PASSWORD, DB_NAME);
      // use inherited attrbute to check result:
      if($this->connect_error) {
         throw new Exception($this->connect_error);
      }
   }
   /**
    * Execute a query and return result
    * @return array (or FALSE on SQL failure)
    * @param string $sql The query to be processed
    */
   public function db_query($sql)
   {
      $result = mysqli_query($this->connection, $sql);
      if (!$result) {
         die("Database query failed: " . mysqli_error($this->connection));
      }
      // added this bit so that external code does not have to worry about
      // database specifics:
      $data = array();
      while(($row = $result->fetch_assoc()) != false) {
         $data[] = $row;
      }
      return $row;
   }
}

User class:

/**
 * User class uses the MySqlDatabase object
 */
class User
{
   /**
    * @var object MySqlDatabase
    */
   private $database;
   /**
    * Constructor
    * @return void
    * @param object $database MySqlDatabase
    */
   public function __construct(MySqlDatabase $database) // use type-hinting to enforce
   {
      $this->database = $database;
   }
   /**
    * Find all users in DB
    * @return array
    */
   public function find_all()
   {
      // since all the work is not in DB class, this is simply:
      return $this->database->db_query("SELECT * FROM users");
   }
   /**
    * Find one user by ID
    * @return array (or false on error)
    * @param integer $id
    */
   public function find_by_id($id = 0)
   {
      // since all the work is not in DB class, this is simply:
      return $this->database->db_query("SELECT * FROM users WHERE id={$id}");
   }
}

Here's a sample usage within your main script:

// instantiate $database object for later use where needed. Have to catch exception
// if it hapens.
try {
   $database = new MySqlDatabase();
} 
catch(Exception $e) {
   error_log((string)$e);
   die("Unable to connect to database");
}

// instantiate $user object, passing it the $database object for its use
$user = new User($database);

// now we can use $user:
$found = $user->find_by_id(1);

// show result:
echo "<pre>";
var_dump($found);
echo "</pre>";

[naike]

NogDog;10965889 wrote:

See if this makes sense to you. I've done a fair bit of rewriting, but tried to keep your original intent.

MySqlDatabase class:

/**
 * Database class using MySQLi
 */
class MySqlDatabase extends MySQLi
{
   // no need for a connection class variable if we do it this way //

   /**
    * Constructor
    * Requires that the constants DB_SERVER, DB_USERNAME, DB_PASSWORD, and
    * DB_NAME be defined already, else throws exception
    * @return void
    */
   function __construct()
   {
      if(!defined('DB_SERVER') or !defined('DB_USERNAME') or
          !defined('DB_PASSWORD') or !defined('DB_NAME')) {
         throw new Exception('Missing one or more DB constant');
      }
      // call the MySQLi __construct() method using the constants:
      parent::__construct(DB_SERVER, DB_USERNAME, DB_PASSWORD, DB_NAME);
      // use inherited attrbute to check result:
      if($this->connect_error) {
         throw new Exception($this->connect_error);
      }
   }
   /**
    * Execute a query and return result
    * @return array (or FALSE on SQL failure)
    * @param string $sql The query to be processed
    */
   public function db_query($sql)
   {
      $result = mysqli_query($this->connection, $sql);
      if (!$result) {
         die("Database query failed: " . mysqli_error($this->connection));
      }
      // added this bit so that external code does not have to worry about
      // database specifics:
      $data = array();
      while(($row = $result->fetch_assoc()) != false) {
         $data[] = $row;
      }
      return $row;
   }
}

User class:

/**
 * User class uses the MySqlDatabase object
 */
class User
{
   /**
    * @var object MySqlDatabase
    */
   private $database;
   /**
    * Constructor
    * @return void
    * @param object $database MySqlDatabase
    */
   public function __construct(MySqlDatabase $database) // use type-hinting to enforce
   {
      $this->database = $database;
   }
   /**
    * Find all users in DB
    * @return array
    */
   public function find_all()
   {
      // since all the work is not in DB class, this is simply:
      return $this->database->db_query("SELECT * FROM users");
   }
   /**
    * Find one user by ID
    * @return array (or false on error)
    * @param integer $id
    */
   public function find_by_id($id = 0)
   {
      // since all the work is not in DB class, this is simply:
      return $this->database->db_query("SELECT * FROM users WHERE id={$id}");
   }
}

Here's a sample usage within your main script:

// instantiate $database object for later use where needed. Have to catch exception
// if it hapens.
try {
   $database = new MySqlDatabase();
} 
catch(Exception $e) {
   error_log((string)$e);
   die("Unable to connect to database");
}

// instantiate $user object, passing it the $database object for its use
$user = new User($database);

// now we can use $user:
$found = $user->find_by_id(1);

// show result:
echo "<pre>";
var_dump($found);
echo "</pre>";

I'm afraid I don't understand all of your code, you're using some functions and stuff I haven't seen before.

Maybe I should just watch some oop tutorials to understand, why it doesn't work..

[naike]

Aah, I now understand your code.

Only problem is that I get an error:

Catchable fatal error: Argument 1 passed to User::__construct() must be an instance of MySqlDatabase, none given (in user class).

<?php
include_once $_SERVER['DOCUMENT_ROOT'] . "/includes/class_database.php";
?>
<?php

class User {

private $database;

public     function __construct(MySqlDatabase $database) { // Using type hinting
$database = $this->database;
}

public static function find_all() {
$result = $database->db_query("SELECT * FROM users");
return $result;    
}

public static function find_by_id($id=1) {
$result = $database->db_query("SELECT * FROM users WHERE id={$id}");
$final = mysqli_fetch_array($result);
return $final;    
}
}
?>

<?php
include_once $_SERVER['DOCUMENT_ROOT'] . "/includes/values.php";
include_once $_SERVER['DOCUMENT_ROOT'] . "/includes/class_user.php";
?>

<?php
class MySqlDatabase extends MySQLi {

function __construct() {
    //Check if constants are missing
    if (defined(!DB_USERNAME) || defined(!DB_SERVER) || 
        defined(!DB_PASSWORD) || defined(!DB_NAME)) {
        die("One or more of the database constants are missing: " . mysqli_error($this->connection));
        }

    //Establish connection if constants are present using the parent class
    parent::__construct(DB_SERVER, DB_USERNAME, DB_PASSWORD, DB_NAME);

    //Echo error message if connection has failed
    if ($this->connect_errno) {
        die("Database connection has failed: " . $this->connect_errno);
        }


}

public function db_query($sql) {
$result = mysqli_query($this-connection, $sql);
    if (!$result) {
        die("Database query failed: " . $this->errno);
    }
return $result;

}

public function query_prep($value) {
$result = mysqli_real_escape_string($this->connection, $value);
    if (!$result) {
        die("Preparing query failed: " . $this->errno);
    }
return $result;
}

// public function db_close() {
// if (isset($this->connection)) {
    // mysqli_close($this->connection);
    // unset($this->connection);
    // }
// }    
}

Calling this way:

<?php
$user = new User();
$found = $user->find_by_id(1);
echo $found;
?>

[NogDog]

Well, you do need to get a good grasp of the basics of OOP if you're going to use it in any way truly object-oriented. It takes most of us a bit of study and practice before we "get it", and then you grow to really like it. I put together a few links for getting started with PHP OOP in a blog article (no videos, I'm afraid). Here are the links, in order:

The basic concepts (not specific to PHP)
PHP OOP basics
More advanced PHP OOP

[naike]

NogDog;10965901 wrote:

Well, you do need to get a good grasp of the basics of OOP if you're going to use it in any way truly object-oriented. It takes most of us a bit of study and practice before we "get it", and then you grow to really like it. I put together a few links for getting started with PHP OOP in a blog article (no videos, I'm afraid). Here are the links, in order:

The basic concepts (not specific to PHP)
PHP OOP basics
More advanced PHP OOP

I read some of those articles, and I didn't really learn anything new.
I just need to use the code so that I understand it fully.
However, after checking php.net for the typecasting you used, in my eyes I should get no error.
php.nets reference manual is pretty useless for everyone except those who already know php well...

Can anyone help me debug this error?
I've already tried several ideas that came to mind but they just caused more errors :S

[bradgrafelman]

NogDog already gave you the answer - compare his sample usage to how you're using the User class.

Or, you could simply read the error message PHP is giving you:

Catchable fatal error: Argument 1 passed to User::__construct() must be an instance of MySqlDatabase, none given (in user class).

So argument #1 was supposed to be an instance of the MySqlDatabase class and yet you didn't pass it anything.

[NogDog]

Yes, the idea if you do it the way I'm suggesting is that you have to pass a database object to the User class when you instantiate it. While your first thought might be that it makes more sense for the User class to instantiate it itself, if you have to explicitly pass it to the User class's constructor, then (a) you can use the same database object with multiple objects, and (b) it makes the User class's dependencies very visible and, hopefully, therefore easier to maintain. as opposed to having that dependency "hidden" within the details of the class's methods.

[naike]

NogDog;10965941 wrote:

Yes, the idea if you do it the way I'm suggesting is that you have to pass a database object to the User class when you instantiate it. While your first thought might be that it makes more sense for the User class to instantiate it itself, if you have to explicitly pass it to the User class's constructor, then (a) you can use the same database object with multiple objects, and (b) it makes the User class's dependencies very visible and, hopefully, therefore easier to maintain. as opposed to having that dependency "hidden" within the details of the class's methods.

If I understood type hinting correctly, this part of code:

<?php
public    function __construct(MySqlDatabase $database) {
?>

basically instantiates $database with the class MySqlDatabase, and the User class now can use it.
At least that's what I first thought, however after getting the error I don't understand why it does not work.
Even after instantiating $database with the mysqldatabase object "manually" it didn't work.

This is how I'm trying to call it now, but still the same thing:

<?php
$database = new MySqlDatabase();
$user = new User($database);
$found = $user->find_by_id(1);
?>

[naike]

Edit:
Fatal error: Call to a member function db_query() on a non-object in user class.

This is the new message.

[naike]

Okay sorry for triple posting but I can't edit my posts...
I figured that this part was wrong after googling some troubleshooting, and it makes sense.

Instead of using something like this:

<?php$result = $this->db_query("SELECT * FROM users WHERE id={$id}"); ?>

I have to do this:

<?php$result = $this->[B]database[/B]->db_query("SELECT * FROM users WHERE id={$id}"); ?>

Anyway, now my error message is telling me my db_query is not correct:

<?php
    public function db_query($sql) {
    $result = mysqli_query($this->connection, $sql);
        if (!$result) {
            die("Database query failed: " . $this->errno);
        }
    return $result;

}
?>

Error message is:
Warning: mysqli_query() expects parameter 1 to be mysqli, null given in ...\includes\class_database.php on line 28

I don't understand how mysqli_query makes my SQL commands = to null.
I'm calling a method in my user class, which then inserts a string "SELECT * FROM users" as an argument into my db_query method, which then passes the string into mysqli_query(), but it doesn't work.

[jgetner]

move the bold statements they are useless in this situation.

Also you sometimes have to return values and in your connect function you do not return $connection.

the best advice start over sorry i now it sounds tough but you learned some stuff here now use that stuff you learned along with testing all your variables and function and make sure they do and return everything you want them to once your done with that then move onto the next variable or function etc... Test driven development helps you learn helps you code better and helps you not have to repair or search for many errors or problems .

[naike]

I don't have to return connect and obviously it is working fine since no errors are returned, also I have tested that function and it's working fine.
My class extends mysqli if you didn't notice.

[bradgrafelman]

naike;10965988 wrote:

it is working fine since no errors are returned

Er... what? Didn't you just post an error message above?

naike wrote:

I don't understand how mysqli_query makes my SQL commands = to null.

The error message has nothing to do with the SQL query string. Read the message again.

[NogDog]

By using mysqli_query() in there, you are using the non-object-oriented version of that command. If you are using my suggestion of simply extending the MySQLi class, then you can instead use the object-oriented version, which is in the format $mysqli_object_name->query($query_string) (no connection parameter). Since your mysqli_object_name is the current object itself, you just use $this. So, that's a long-winded way of saying that, instead of this...

$result = mysqli_query($this->connection, $sql);

...use this...

$result = $this->query($sql);

(If, on the other hand, you are not using my suggested design, then never mind.)

[naike]

NogDog;10965993 wrote:

By using mysqli_query() in there, you are using the non-object-oriented version of that command. If you are using my suggestion of simply extending the MySQLi class, then you can instead use the object-oriented version, which is in the format $mysqli_object_name->query($query_string) (no connection parameter). Since your mysqli_object_name is the current object itself, you just use $this. So, that's a long-winded way of saying that, instead of this...

$result = mysqli_query($this->connection, $sql);

...use this...

$result = $this->query($sql);

(If, on the other hand, you are not using my suggested design, then never mind.)

Hi.
Thanks, I went everything through again, and I understand what your point was now.
Sorry but I started learning php a week ago or so so pardon me 🙂
Now, one last question to annoy you with.

I'm planning on using this function to make user entered content like usernames etc SLQ ready:

<?php
    public function query_prep($value=) {
    $result = mysqli->real_escape_string($value);
        if (!$result) {
            die("Preparing query failed: " . $this->errno);
        }
    return $result;
}
?>

However, even without calling this method, I'm getting this error:
Parse error: syntax error, unexpected T_OBJECT_OPERATOR in C:\xampp\htdocs\includes\class_database.php on line 35

I've tried googling some solutions, but the majority were just Php4 incompatibility issues. Any ideas?

[NogDog]

Again, $this will be your friend. 😉 Also, you'll want to check if the result "is identical" to boolean false (via the "===" or "!==" operators), not just "not equal" (the "!" or "!=" or "==" operators), as otherwise you will get false negative results on values such as "0" or an empty string. This is because of PHPs "loose typing", which normally converts types on the fly as needed, except when explicitly told not to.

public function query_prep($value=) {
    $result = $this->real_escape_string($value);
        if ($result === false) {
            die("Preparing query failed: " . $this->errno);
        }
    return $result; 
}

[naike]

NogDog;10966039 wrote:

Again, $this will be your friend. 😉 Also, you'll want to check if the result "is identical" to boolean false (via the "===" or "!==" operators), not just "not equal" (the "!" or "!=" or "==" operators), as otherwise you will get false negative results on values such as "0" or an empty string. This is because of PHPs "loose typing", which normally converts types on the fly as needed, except when explicitly told not to.

public function query_prep($value=) {
    $result = $this->real_escape_string($value);
        if ($result === false) {
            die("Preparing query failed: " . $this->errno);
        }
    return $result; 
}

Thanks I've got almost everything working til now.
Just, I have a method with 1 argument.
I insert an array into the method, it then does the real escape string thing on each item, and using array_map puts it back together, but when I return it, I get nothing but emptiness.

If i print_r it just before returning it works fine, but once returned I can't use it in my index.php, its just empty.