Retrieving result sets from a MySQL table 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-&gt;prepare($sql)){
        $x = 1;
        $vec = [];
        if(count($params)){
            foreach($params as $p){
                $stmt-&gt;bindValue($x, $p);
                $x++;
            }
        }
    }
    if($stmt-&gt;execute()){
        while($row = $stmt-&gt;fetch(PDO::FETCH_OBJ)){
            $array = '';
            foreach ($cols as $col) {
                $array .= $row-&gt;$col.' ';
            }
            $vec[] = $array;
        }
        return $vec;
    } else {
        return $db-&gt;errorInfo();
    }
}


Call:

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


My questions are the following:


How can I improve this function?
Is there any query that can not be run correctly by this function? joins, subqueries, etc.
Is there anything else you think that isn't right at this function?
The first thing I've noticed is that you return 2 different things:


The data that the user wanted;
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-&gt;execute()){
    while($row = $stmt-&gt;fetch(PDO::FETCH_OBJ)){
        $array = '';
        foreach ($cols as $col) {
            $array .= $row-&gt;$col.' ';
        }
        $vec[] = $array;
    }
    return $vec;
} else {
    return $db-&gt;errorInfo();
}


Use early returns, like this:

if(!$stmt-&gt;execute()){
    return null;
}
while($row = $stmt-&gt;fetch(PDO::FETCH_OBJ)){
    $array = '';
    foreach ($cols as $col) {
        $array .= $row-&gt;$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-&gt;prepare($sql)){
    $x = 1;
    $vec = [];
    if(count($params)){
        foreach($params as $p){
            $stmt-&gt;bindValue($x, $p);
            $x++;
        }
    }
}


Applying the early return:

$stmt = $db-&gt;prepare($sql)
if(!$stmt){
    return null;
}

$i = 1;
$vec = [];

foreach($params as $param){
    $stmt-&gt;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 =&gt; $param ) {
    $stmt-&gt;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-&gt;bindValue($x, $p);
        $x++;
    }
}


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

foreach($params as $param){
    $stmt-&gt;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-&gt;fetch(PDO::FETCH_OBJ)){
    $array = '';
    foreach ($cols as $col) {
        $array .= $row-&gt;$col.' ';
    }
    $vec[] = $array;
}
return $vec;


Question time:


What, in the name of the Lord, is going on here?
Why are you fetching an object instead of an associative array?
What does $array do?
Why is it called $array?
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-&gt;$col.' ';.

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

//test variable, fakes the database
$items = array(
    (object)array(
        'b'=&gt;5,
        'c'=&gt;'a'
    ),
    (object)array(
        'b'=&gt;7
    )
);

$cols = array('b','c');//only 1 column

//simulates $stmt-&gt;fetch(PDO::FETCH_OBJ)

while($row = array_shift($items)){
    $array = '';
    foreach ($cols as $col) {
        $array .= $row-&gt;$col.' ';
    }
    $vec[] = $array;
}
var_export($vec);


Result I expected (added extra whitespace):


array (
    0 =&gt; 
    array (
        'b' =&gt; 5,
        'c' =&gt; 'a',
    ),
    1 =&gt; 
    array (
        'b' =&gt; 7,
    ),
)



Result obtained:


&lt;br /&gt;
&lt;b&gt;Notice&lt;/b&gt;:  Undefined property: stdClass::$c in &lt;b&gt;[...][...]&lt;/b&gt; on line &lt;b&gt;7&lt;/b&gt;&lt;br /&gt;
array (
    0 =&gt; '5 a ',
    1 =&gt; '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.