Retrieving result sets from a MySQL table

Question

This retrieves result sets from a MySQL table. The objective of this function is to correctly retrieve any number of rows (with $sql) from a MySQL table, binding the needed values accordingly ($params[]), having the possibility to choose the column/s you want to retrieve ($cols[]).

$db = new PDO('mysql:host=127.0.0.1;dbname=xxx;charset=utf8','root','');
function query($sql, $params=[], $cols=[], $db){
 if($stmt = $db->prepare($sql)){
 $x = 1;
 $vec = [];
 if(count($params)){
 foreach($params as $p){
 $stmt->bindValue($x, $p);
 $x++;
 }
 }
 }
 if($stmt->execute()){
 while($row = $stmt->fetch(PDO::FETCH_OBJ)){
 $array = '';
 foreach ($cols as $col) {
 $array .= $row->$col.' ';
 }
 $vec[] = $array;
 }
 return $vec;
 } else {
 return $db->errorInfo();
 }
}

Call:

foreach (query('SELECT * FROM angajati WHERE nume=?',['admin'],['id','nume','prenume'],$db) as $q){
 echo $q,'<br/>';
}

My questions are the following:

1. How can I improve this function?
2. Is there any query that can not be run correctly by this function? joins, subqueries, etc.
3. Is there anything else you think that isn't right at this function?

• Ismael Miguel: Can you provide an example of the output?

Answer 1

The first thing I've noticed is that you return 2 different things:

1. The data that the user wanted;
2. The error information.

Now, how will you distinguish between an error-ed query and a successful one? You can't! And that makes angels angry and cry.

Return null in case of error.

---

Your variable names are really....... conflicting and incomplete...

$x = 1; //Should be $i
$vec = []; //No idea what this is for
foreach($params as $p) //$p should be $param
$array = ''; // ... a variable called $array that receives a string ... WTH!?

A good variable name will give us an idea of it's content without having to read through all the code.
The name for $i (instead of $x) is the standard name used for an increment. Since you have $x++ below, it is a good name.

---

Now, you have this block:

if($stmt->execute()){
 while($row = $stmt->fetch(PDO::FETCH_OBJ)){
 $array = '';
 foreach ($cols as $col) {
 $array .= $row->$col.' ';
 }
 $vec[] = $array;
 }
 return $vec;
} else {
 return $db->errorInfo();
}

Use early returns, like this:

if(!$stmt->execute()){
 return null;
}
while($row = $stmt->fetch(PDO::FETCH_OBJ)){
 $array = '';
 foreach ($cols as $col) {
 $array .= $row->$col.' ';
 }
 $vec[] = $array;
}
return $vec;

This helps the code to be more readable and reduces the nesting level. Too many chained ifs and loops will cause your code to be hard to read.

---

You have the following if, just a little bellow:

if($stmt = $db->prepare($sql)){
 $x = 1;
 $vec = [];
 if(count($params)){
 foreach($params as $p){
 $stmt->bindValue($x, $p);
 $x++;
 }
 }
}

Applying the early return:

$stmt = $db->prepare($sql)
if(!$stmt){
 return null;
}
$i = 1;
$vec = [];
foreach($params as $param){
 $stmt->bindValue($i, $param);
 $i++;
}

See? It is so much easier to read!

In fact, the loop can be like that: (this point is subjective!)

foreach( array_keys($params) as $i => $param ) {
 $stmt->bindValue($i + 1, $params[$param]);
}

Bye bye $i++;!

---

Going deeper on your code, I see that you also have some useless code.

Check this block:

if(count($params)){
 foreach($params as $p){
 $stmt->bindValue($x, $p);
 $x++;
 }
}

Why do you have that count() there? It does nothing there. Just do this:

foreach($params as $param){
 $stmt->bindValue($i, $param);
 $i++;
}

The foreach doesn't care about the length of $params. As long as it is an array.

---

And now, going to this block:

while($row = $stmt->fetch(PDO::FETCH_OBJ)){
 $array = '';
 foreach ($cols as $col) {
 $array .= $row->$col.' ';
 }
 $vec[] = $array;
}
return $vec;

Question time:

1. What, in the name of the Lord, is going on here?
2. Why are you fetching an object instead of an associative array?
3. What does $array do?
4. Why is it called $array?
5. What is $vec for? What's in it?

After testing myself, I concluded this: It's bugged!!!
The expected result is different that the provided result.
I really don't get the reasoning to do $row->$col.' ';.

Here's a test code I've build, based on yours:

//test variable, fakes the database
$items = array(
 (object)array(
 'b'=>5,
 'c'=>'a'
 ),
 (object)array(
 'b'=>7
 )
);
$cols = array('b','c');//only 1 column
//simulates $stmt->fetch(PDO::FETCH_OBJ)
while($row = array_shift($items)){
 $array = '';
 foreach ($cols as $col) {
 $array .= $row->$col.' ';
 }
 $vec[] = $array;
}
var_export($vec);

Result I expected (added extra whitespace):

array (
 0 => 
 array (
 'b' => 5,
 'c' => 'a',
 ),
 1 => 
 array (
 'b' => 7,
 ),
)

Result obtained:

<br />
<b>Notice</b>: Undefined property: stdClass::$c in <b>[...][...]</b> on line <b>7</b><br />
array (
 0 => '5 a ',
 1 => '7 ',
)

_{Check it on http://sandbox.onlinephpfunctions.com/code/1e0527eccba4ada6de7b4cdba9967ab9df4e2ce7}

---

In conclusion: The code is broken! Fixing it is left as an exercise for you.

• Dan Costinel: Thanks! I guess I should not answer with this kind of words, but I do really appreciate your answer! Now: - Why are you fetching an object instead of an associative array? - I'm fetching object because I will use those functions into a class. - What does $array do? - $array is an array containing the column names I want that query to retrieve (those ones ['id','nume','prenume']) - Why is it called $array? - I'm bad at finding relevant names :( - What is $vec for? What's in it? - vec[] is the result set - contains each row returned by that query.
• Ismael Miguel: @DanCostinel 1- You could use PDO::FETCH_ASSOC instead, which returns an array. 2- $array actually has a string. And not an $array. From what I understand, it contains the concatenation of the values from the columns. 3- $vec seems to be an array containing an array of $arrays. If my wording is confusing you, I'm really sorry. But also, I recommend that you don't mark as accepted so soon.
• Dan Costinel: Ok! I accepted your answer, because it was fast, and clear (with examples). If others will come with another good answer, it will have my upvote. Since then: I guess you got those errors, because there is not column name in the db you've working on (if any) with that name ('c'). Am I right? And for that while($row = stmt->fetch(PDO::FETCH_OBJ)){ ... } - between { } we normally return the column names we want, isn't so? But I want my return only some columns,or all - the purpose is to obtain only the columns that were wrote in the 2nd array (when calling). And yes, $array is a string :(
• Ismael Miguel: @DanCostinel Actually, what you will want is what is on the expected result. If it crams up all values in a string, it isn't useful. And yes, it is because the 2nd object didn't have the 'c'.
• Dan Costinel: So any idea on how to not cram all values in a string? This for sure can be a great improvement. Some examples: SELECT * FROM tbl WHERE col1=? AND col1 LIKE ? ORDER BY ? ASC - with this query, maybe at some point I want to return 1 column, and at another point I want to return (with the same query), 10 columns - from the same table. This is what I want to accomplish with this function.