0
\$\begingroup\$

I am not very experienced in PHP OOP. Moreover, when MySQLi comes with PHP OOP, it makes me more confused about using it in the most efficient way.

Connection class:

<?php
//connectionclass.php
class connection{
public $conn;
public $warn;
public $err;

function __construct(){
  $this->connect();
}

private function connect(){
  $this->conn = @ new mysqli('localhost', 'sever_user', 'user_password');
    if ($this->conn->connect_error) {
      $this->conn = FALSE;
      $this->warn = '<br />Failed to connect database! Please try again later';
     }
}

public function get_data($qry){
  $result = $this->conn->query($qry);
  if($result->num_rows>=1){
    return $result;
  }else{
    $this->err = $this->conn->error;
    return FALSE;
  }
  $result->free();
}

public function post_data($qry){
  $this->conn->query($qry);
  if($this->conn->affected_rows>=1){
    return TRUE;
  }else{
    $this->err = $this->conn->error;
    return FALSE;
  }
}
}
?>

Structure of a PHP page which uses a MySQL database to store and get data:

<?php
//login.php
include('/include/connectionclass.php');
$db = new connection();

$query = "SELECT * FROM USERS WHERE user_country='India'";
$data = $db->get_data($query);
  if($data){
    while($row=$data->fetch_assoc()){
      echo 'User Name: ':.$row["user_name"].' Age: '.$row["age"];
    }
  }
?>

My login.php uses a connection class to get data about users. All the things are running well, but one thing confused me. In connectionclass.php, $this->conn is itself an object as it calls new mysqli, so this is an object inside another object $db. Moreover, When I am using $data = $db->get_data($query);, a result set is created inside a $db object by method get_data, then this result set is copied into a variable $data inside the login.php page.

Actually, two instances of the same result sets/data sets are creating here, one inside the db object and one inside the login page.

Is this the right way to use MySQLi and PHP to get a dataset from a MySQL database? Will it use more memory and server resources when the dataset is larger (when have to get large amount of data for many users)?

If it is not right way, please explain your points and provide another solution which can be used efficiently for PHP OOP and MySQLi.

\$\endgroup\$
1
\$\begingroup\$

No, this isn't the right way to use OOP or MySQLi.

First of all, you only provide two methods to access your wrapped mysqli instance: one to get data, one to post data.

This hides a lot of useful and necessary functions. You can't even use prepared statements with your code, making it incredibly insecure.

You also didn't really gain anything with your functions. With your functions, your code is:

$query = "SELECT * FROM USERS WHERE user_country='India'";
$data = $db->get_data($query);
  if($data){
    while($row=$data->fetch_assoc()){
      echo 'User Name: ':.$row["user_name"].' Age: '.$row["age"];
    }
  }

Without your functions it would be:

$query = "SELECT * FROM USERS WHERE user_country='India'";
$data = $db->query($query);
  if($data){
    while($row=$data->fetch_assoc()){
      echo 'User Name: ':.$row["user_name"].' Age: '.$row["age"];
    }
$data->free();

It's essentially the same code, making your functions unnecessary.

If you do want to wrap mysqli - which may not actually be necessary in your case -, make sure that you:

  • provide all the needed functionalities.
  • actually improve mysqli (make it easier to use, add functionalities, ...).
  • keep the interface as close to that of mysqli as possible, to make it easier to use.

Instead of wrapping mysqli, it may make sense to aim for less abstract methods. For example, you may have a UserDAO, which may have methods such as selectByCountry.

\$\endgroup\$
  • \$\begingroup\$ Dear tim, I used class method get_data in order to ensure that if there is atleast one row or more in result set, then the while statement will be executed. But if I use without method (your 2nd chunk of code), then even if there is no matching rows, it will be considered that the query is executed successfully (as there is no error in code) and the while statement will be executed even there is no rows in returned result set. Am I right? \$\endgroup\$ – Abdullah Mamun-Ur- Rashid May 23 '16 at 14:36
  • \$\begingroup\$ @AbdullahMamun-Ur-Rashid in that case the if would just need to be changed to if ($data->num_rows>=1). Either way, it's essentially the same, your method really only adds the free call. If you consider all the downsides of it (developers are used to the mysqli API, not your methods; essential methods are missing; etc), I don't think that it's worth having this method. \$\endgroup\$ – tim May 23 '16 at 17:00
  • \$\begingroup\$ Dear Tim, Thanks for your reply. Please keep it mind that I myself is not satisfied with my code, that's why I began this question to this forum. My objective is not to defend you but to understand the background reason from different point of view. Please make me clear for one point. If I use the class and method (get_data), then if the query is not executed, I can get error by that method, I don't have to write code every time to get the error. But If I use direct query, I have to write else statement everytime to collect error. How to overcome this for direct query/mysqli API? \$\endgroup\$ – Abdullah Mamun-Ur- Rashid May 23 '16 at 17:19
  • \$\begingroup\$ @AbdullahMamun-Ur-Rashid I'm not sure that I see the difference. You either access the error variable of your custom class, or of mysqli directly, but either way, you have to first check if it's a new error message or an old one. I don't see the advantage of your approach. Maybe I'm overlooking something obvious here, but it doesn't really matter either way, as your code is currently too limited to be used in practice (you can't use prepared statements with it, and are thus vulnerable to SQL injection as soon as you need variable input in your queries). \$\endgroup\$ – tim May 23 '16 at 17:31
  • \$\begingroup\$ Tim, I have omitted escaping methods here for the sake of keeping my sample code simple. Injection can be prevented from the class method too, that doesn't matter. But I have to write if($data){------}else{ $error[]=$db->error;} for every database query. Would you tell me one way so that I don't have to write $error[]=$db->error for every database query for mysqli API? \$\endgroup\$ – Abdullah Mamun-Ur- Rashid May 23 '16 at 17:39
1
\$\begingroup\$

@Raoul Mensink's answer is limited to only fetching one row, to get more than one row in that class the code can be changed to this:

   public function Fetch($qry)
    {
        $result = $this->conn->query($qry);
        if ($result->num_rows>=1) {
            while($row=$result->fetch_assoc()){
               $rows[] = $row;
            }
            return $rows;
        } else {
            $this->err = $this->conn->error;
            return FALSE;
        }
    }

and then iterate through the rows in php page like this:

$rownum = count($rows);
for($i=0;$i<$rownum;$i++){
    echo $rows[$i]['name'].'- '.$rows[$i]['age'].'<br />';
}
\$\endgroup\$
  • \$\begingroup\$ Your answer was posted more as a comment to Raoul Mensik's answer, I edited it to make it be more like an answer. \$\endgroup\$ – Simon Forsberg Dec 3 '16 at 10:39
0
\$\begingroup\$
class db
{
    private $conn;
    public $warn;
    public $err;

    function __construct()
    {
        $this->conn = @ new mysqli('localhost', 'sever_user', 'user_password');
        if ($this->conn->connect_error) {
            $this->conn = FALSE;
            $this->warn = '<br />Failed to connect database! Please try again later';
        }
    }

    public function Fetch($qry)
    {
        $result = $this->conn->query($qry);
        if ($result->num_rows>=1) {
            return $result;
        } else {
            $this->err = $this->conn->error;
            return FALSE;
        }
    }

    public function Insert($qry)
    {
        $this->conn->query($qry);
        if ($this->conn->affected_rows > 0) {
            return TRUE;
        } else {
            $this->err = $this->conn->error;
            return FALSE;
        }
    }
}

So I changed it a bit:

  1. added indenting
  2. removed your private connection function
  3. changed names

What I would Change on your structure. return the rows instead of a mysqli object. You are asking for rows, give them directly not a sublayer

    public function Fetch($qry)
    {
        $result = $this->conn->query($qry);
        if ($result->num_rows>=1) {
            return $result->fetch_assoc();
        } else {
            $this->err = $this->conn->error;
            return FALSE;
        }
    }

Also throw an error when your get a database error or check if $conn is true

\$\endgroup\$
  • \$\begingroup\$ Raoul thanks for your comment. If I fetch rows inside class object, how it could be possible to get if there are more than one rows in the result set? \$\endgroup\$ – Abdullah Mamun-Ur- Rashid May 23 '16 at 14:28
  • \$\begingroup\$ you do know that the function fetch_assoc() gets all rows right? your while simply looped through that array \$\endgroup\$ – Raoul Mensink May 24 '16 at 7:01
  • \$\begingroup\$ What is demerits if use mysqli object instead using rows directly. Both have to iterate to use the result (e.g to create html table rows) \$\endgroup\$ – Abdullah Mamun-Ur- Rashid Nov 26 '16 at 14:27
  • \$\begingroup\$ @AbdullahMamun-Ur-Rashid the demerits are you Keep rebuilding an database Connection, this creates double code and inconsistancy between database Connections. If you made one small mistake you will Need to check every file if they got that as well instead of haveing an centralised class that is being referenced. \$\endgroup\$ – Raoul Mensink Nov 28 '16 at 8:49

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.