2

Trying to get a function working to create simple CRUD "Select" with multiple parameters to any table. I think I got the hardest part, but couldn't fetch the data right now. Maybe I'm doing something wrong I can't figure out.

My prepared statement function:

function prepared_query($mysqli, $sql, $params, $types = ""){
    $types = $types ?: str_repeat("s", count($params));
    if($stmt = $mysqli->prepare($sql)) { 
        $stmt->bind_param($types, ...$params);
        $stmt->execute();
        return $stmt;
    } else {
        $error = $mysqli->errno . ' ' . $mysqli->error;
        error_log($error);
    }
}

The query creator:

function create_select_query($table, $condition = "", $sort = "", $order = " ASC ", $clause = ""){
    $table = escape_mysql_identifier($table);
    $query = "SELECT * FROM ".$table;
    if(!empty($condition)){
        $query .= create_select_query_where($condition,$clause);
    }
    if(!empty($sort)){
        $query .= " ORDER BY ".$sort." $order";
    }
    return $query;
}

The helper function to create the WHERE clause:

function create_select_query_where($condition,$clause){
    $query          = " WHERE ";
    if(is_array($condition)){
        $pair       = array();
        $size       = count($condition);
        $i = 0;
        if($size > 1){
            foreach($condition as $field => $val){
                $i++;
                if($size-1 == $i){
                    $query .= $val." = ? ".$clause. " ";
                }else{
                    $query .= $val." = ? ";
                }
            }        
        }else{
            foreach($condition as $field => $val){
                $query .= $val." = ? ";
            }
        }
    }else if(is_string($condition)){
        $query .= $condition;
    }else{
        $query = "";
    }
    return $query;
}

The select function itself:

function crud_select($conn, $table, $args, $sort, $order, $clause){
    $sql = create_select_query($table, array_keys($args),$sort, $order, $clause);
    print_r($sql);
    if($stmt = prepared_query($conn, $sql, array_values($args))){
        return $stmt;
    }else{
        $errors [] = "Something weird happened...";
    } 
}

When I create the query, it seems to be OK but can't fetch the data. If I create an array with only one argument the query translates into:

SELECT * FROM `teste_table` WHERE id = ?

If I create with multiple parameters, it turns like this:

SELECT * FROM `teste_table` WHERE id = ? AND username = ?

So, how can I properly fetch the data from the select. This should be used for multiple purposes, so I could get more than one result, so the best way would be fetch data as array I guess.

I guess I'm close, but can't figure it out. Thanks

Tiago
  • 625
  • 5
  • 16
  • 1
    You should listen to YCS. You are creating an incomprehensible functions which add no value. As a learning experience it might be good for you, but they are utterly unsuable and if you did want to use it you would hurt yourself only in the process. – Dharman Sep 11 '19 at 15:20
  • I updated my answer to provide a more flexible solution – Your Common Sense Mar 04 '23 at 08:53

1 Answers1

4

In PHP >= 8.2 there is a function that lets you execute a SELECT query (as well as any other kind of query) in one go, execute_query():

$query = 'SELECT Name, District FROM City WHERE CountryCode=? ORDER BY Name LIMIT ?';
$rows = $mysqli->execute_query($query, ['DEU', 5])->fetch_all(MYSQLI_ASSOC);

For the older versions you will need a helper function like this:

function prepared_query($mysqli, $sql, $params, $types = ""){
    $types = $types ?: str_repeat("s", count($params));
    $stmt = $mysqli->prepare($sql)) { 
    $stmt->bind_param($types, ...$params);
    $stmt->execute();
    return $stmt;
}

Just stick to it and it will serve you all right.

$sql = "SELECT * FROM `teste_table` WHERE id = ? AND username = ?";
$stmt = prepared_query($mysqli, $sql, [$id, $name]);
$row = $stmt->get_result()->fetch_assoc();

Fora a real CRUD, however, you could use the approach called Table Gateway Pattern. For this, you can create a class, that would implement methods for all basic operations on a table:

  • Creating a record (using INSERT query)
  • Reading a record (Using Primary key lookup)
  • Updating a record
  • Deleting a record

Then you would extend this class for the every table used.

And then you will have a CRUD solution that is concise, readable and safe.

Here is a sample Table Gateway I wrote for the occasion

By simply extending the basic class, you can get everything you want in a much simpler code:

// add a class for the table, listing its name and columns explicitly
class UserGateway extends BasicTableGateway {
    protected $table = 'gw_users';
    protected $fields = ['email', 'password', 'name', 'birthday'];
}

// and then use it in your code
$userGateway = new UserGateway($pdo);

$user = $userGateway->read($id);
echo "Read: ". json_encode($user),PHP_EOL;

See - it's really simple, yet safe and explicit.

Remember that for the more complex queries you will have to either use plain SQL or add new methods to the UserGateway class, such as

public function getByEmail($email) {
    return $this->getBySQL("SELECT * from `{$this->table}` WHERE email=?",[$email]);
}

Speaking of your current approach, it's just unusable. I told you to limit your select function to a simple primary key lookup. And now you opened a can of worms. As a result you are getting entangled implementation code and unreadable application code.

$table, $args, $sort, $order, $clause

What all these variables are for? How you're going to call this function - a list of gibberish SQL stubs in a random order instead of plain and simple SQL string? And how to designate a list of columns to select? How to use JOINS? SQL functions? Aliases? Why can't you just write a single SQL statement right away? You already have a function for selects, though without this barbaric error reporting code you added to it:

Your Common Sense
  • 156,878
  • 40
  • 214
  • 345
  • I read your answer very well sir, but we need to learn ourselves. Couldn't understand why I can't make (or try) get better functions that could serve multipruposes. $table is the table name, $args are the arguments to search for, $sort is to add the sort ASC os DESC to query and clause is "AND/OR" for multiple params. I know it could be a pain, but we only learn making things that would difficult us. And the columns to search for are in the arguments array (ex $args['id'] = 1). It will search WHERE id = 1 :) – Tiago Sep 11 '19 at 14:41
  • 1
    Because your crud_select() function is anything but "better". I understand the notion, every learner have to pass that phase, but if you have any respect to tens of years of experience, you can take my word for it: your attempts to use this function will be the *real* pain. – Your Common Sense Sep 11 '19 at 14:44
  • This isn't the "basics" of an "Advanced Search function"? I really can't understand. If you could explain me better why I shouldn't do it, I'd appreciate. Ofc selects will be worst to create, but at least I know why I shouldn't do it. Your reason was "A proper query builder is ten times more complex program than your whole crud, better leave it alone". So, the problem is this isn't optimized and will be worst tha quick SQL query? Will not work with JOINS for example? It's insecure? Just to understand it. Thanks. – Tiago Sep 11 '19 at 14:47
  • Ironically, this function is already non-table agnostic, with all these parameters. For different tables there will be different field names in the order and where parts, so you cannot reuse the same code for different tables, like you can with inserts. So the goal of multiprupose is not reached. Have a nice day. – Your Common Sense Sep 11 '19 at 14:48
  • I guess you didn't understand yet how the functions works. The $args var is an array. That array have in their index table the columns I want to search. The value or that index, is the real value I want to search. If it's working for multiple update or delete, it could also work for select. – Tiago Sep 11 '19 at 14:50
  • 1
    Guess I have seen so much similar functions on Stack Overflow alone that I even wrote a dedicated article about such mistakes, including a [section about this very function](https://phpdelusions.net/pdo/common_mistakes#select). There is no benefit in making $args an array for a select query. You cannot reuse a complex select query like you do with inserts. And you can write your conditions in the raw SQL as well. You'll end up with a calling code that is ten times harder to read than plain SQL with not a single benefit in return. Anyway, this is going nowhere, so I am off.Have a nice day. – Your Common Sense Sep 11 '19 at 15:00
  • Thanks for your explanation. Now it makes sense for me. – Tiago Sep 11 '19 at 16:53