0
\$\begingroup\$

I have a PHP class:

<?php
//connectionclass.php
class connectionclass{
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) {
        while($row=$result->fetch_assoc()){
           $rows[] = $row;
        }
        return $rows;
    } else {
        $this->err = $this->conn->error;
        return FALSE;
    }
}

}
?>

and a PHP page:

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

$query = "SELECT * FROM USERS WHERE user_country='India'";
$data = $db->get_data($query);
  $rownum = count($data);
  for($i=0;$i<$rownum;$i++){
     echo $data[$i]['name'].'- '.$data[$i]['age'].'<br />';
  }
?>

Is there any idea for an easier way to iterate $data in the PHP page?

\$\endgroup\$
2
\$\begingroup\$

First, you can use mysqli_result::fetch_all in your connectionclass::get_data() instead of manually looping and building the result array.

Second, you can use foreach to easily iterate through results

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

The improvement to your previous question is that you are now successfully encapsulating mysqli, instead of exposing part of it by returning mysqli related objects.

I still stand by my previous answer though: Your function still doesn't make it easier to write queries, and it abstracts away essential functionalities of mysqli (ie prepared statements).

With your function, your code looks like this (I cleaned it up a bit):

$data = $db->get_data("SELECT * FROM users WHERE user_country = 'India'");
for($i = 0; $i < count($data); $i++){
    echo $data[$i]['name'] . '- ' . $data[$i]['age'] . '<br />';
}

Without your function, the code would look like this:

$result = $conn->query("SELECT * FROM users WHERE user_country = 'India'");
while($row = $result->fetch_assoc()){
    echo $row['name'] . '- ' . $row['age'] . '<br />';
}

If you want to add error handling, each approach would have additional ifs.

So you can see that you are not actually making it any easier to write code, but you are changing the well known mysqli API to your custom function, and you are making it impossible to write secure code, as prepared statements cannot be used with your function.

So instead of your approach, you need to use prepared statements. If you still want to wrap mysqli, try to do it in a way that actually makes writing queries easier. Note though that the mysqli API is already pretty good, so it will not be that easy to improve upon it.

Misc

  • try to follow coding standards. Your indentation and spacing are off, making your code harder to read.
  • write out your variable names. warning and error are not that much more to type, but easier to read.
\$\endgroup\$
  • \$\begingroup\$ Your conception that these types of class could not implement prepared statement is actually not right. It can also implement prepared statement nicely. For your kind information that prepared statement is not always useful. From php.net manual: Using a prepared statement is not always the most efficient way of executing a statement. A prepared statement executed only once causes more client-server round-trips than a non-prepared statement. This is why the SELECT is not run as a prepared statement above. So if you have a shared hosting with limited resources, you should think alternative. \$\endgroup\$ – Abdullah Mamun-Ur- Rashid Dec 3 '16 at 14:28
  • 1
    \$\begingroup\$ @AbdullahMamun-Ur-Rashid These types of wrapper classes can expose prepared statement functionality, but yours doesn't. If it would, my advise would likely be the same though. The example which is not using prepared statements doesn't use any parameter which would need to be bound, so it wouldn't make sense to use prepared statements. In 99.9% of cases the security benefits of prepared statements far outweigh the negligible or non-existent performance benefit of not using them. \$\endgroup\$ – tim Dec 3 '16 at 15:27
  • \$\begingroup\$ tim, My topics was not for prepared statement, thus it was unnecessary to show in my code, it would complicated my main points. My point in this topic was how to fetch resulting array in an easier way. Xymanek adviced me for usingforeach loop. It \$\endgroup\$ – Abdullah Mamun-Ur- Rashid Dec 3 '16 at 17:01
  • 1
    \$\begingroup\$ @AbdullahMamun-Ur-Rashid I think you are missing the main takeaway here. Whether or not you added the ability to use prepared statements to this class, you have still added a class you need to maintain to your application, and this class really adds no value above working directly with mysqli. \$\endgroup\$ – Mike Brant Dec 4 '16 at 5:27

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.