PHP MySQL GET code review

I think it's quite a simple question. Is this my best bet or is there a 'proper' way of doing this?

<?php  
    $correctOrder = array("name", "address", "phone", "starttime", "endtime", 
                    "status", "details");  

    $sql->sql1 = "SELECT * FROM `pickups` WHERE";  

    if (isset($_GET["name"])){  
        $sql->sql2 = "`name` LIKE '%" . $_GET['name'] . "%'";  
        }  
    if (isset($_GET["address"])){  
        if (!isset($_GET['name'])){
            $q = "`address` LIKE '%" . $_GET['address'] . "%'";  
        } else {  
            $q = "AND `address` LIKE '%" . $_GET['address'] . "%'";  
            }
        $sql->sql3 = $q;
        }
    ...
    ...

    echo implode(" ", (array) $sql);  
?>  

So, right now:

?name=Jari%20Martikainen&address=some%20random%20street

and

?name=Jari%20Martikainen&address=some%20random%20street&blah=har

and

?address=some%20random%20street&blah=har&name=Jari%20Martikainen

all return the same result which is desired, but it just doesn't seem like a very efficient way of doing things.

Upvotes: 0

Views: 98

Answers (3)

Rick James
Rick James

Reputation: 142366

Build an array ($ands) of AND clauses, but without the "AND".

$ands = array();
if (...)
    $ands[] = "name LIKE ...";
if (...)
    $ands[] = "address LIKE ...";
...

Then build the query:

$query = "SELECT ... WHERE " . implode(' AND ', $ands);

I find this pattern to be simple, clean, and avoids kludges like 1=1 or removing the extra AND.

Upvotes: 1

Will Hines
Will Hines

Reputation: 334

Here's a bit more of an overhaul than you asked for.

First, start your WHERE clause with 1=1, then cycle through your array of search terms, and extend the where clause if any of them are included in the $_GET array.

Second, I'd use a PDO object to connect to your database, and then BIND the $_GET values. That will help protect you from SQL injection/hacking stuff.

Below, I cycle through your search terms, saving them to an array called $sqlParams if there's something in the $_GET array. Then I cycle through $sqlParams and bind the values.

// connect to database via PDO
try {
    $db = new PDO($dsn, $username, $password, $options);    
} catch (PDOException $e) {
    echo 'Connection failed: ' . $e->getMessage();
}

$correctOrder = array("name", "address", "phone", "starttime", "endtime", 
                "status", "details");  

$sql = "SELECT * FROM `pickups` WHERE 1=1 ";  

// check the $_GET array, build SQL, remember values in $sqlParams
$sqlParams = [];
foreach ($correctOrder as $key) {
    if (isset($_GET[$key])) {
        $sql .= "and `$key` LIKE :$key ";
        $sqlParams[":$key"] = "%{$_GET[$key]}%";
    }
}


// bind values from $sqlParams into SQL statement
$stmt = $db->prepare($sql);
foreach ($sqlParams as $key => &$value) {
    $stmt->bindParam($key, $value);
}

// execute SQL statement, catching exception
try {
    $stmt->execute();
} catch (PDOException $e) {
    echo 'Connection failed: ' . $e->getMessage();
}

// cycle through results and do stuff
while ($row = $stmt->fetch()) {
    print_r($row);
}

Upvotes: 0

Nigel Ren
Nigel Ren

Reputation: 57131

This code uses an array to work out what the parameters are that it's interested in and builds up an array of where clauses.

The reason why I use "name" => "name" in this array ($correctOrder) is that it allows the parameter name and the column name to be different. You should add any parameters you need in here.

Also this code uses bind variables, not sure what flavour of database access your using, but you can pass the $data array to the execute to bind them.

$correctOrder = array("name" => "name", "address" => "address");

$_GET = [ 'name' => 'name1', 'address' => 'add'];

$where = [];
$data = [];
foreach ( $_GET as $paramName => $param ) {
    if ( isset($correctOrder[$paramName]) ) {
        $where[] = "`{$correctOrder[$paramName]}` like ? ";
        $data[] = '%'.$param.'%';
    }
}

$sql = "SELECT * FROM `pickups`";
if ( count($where) > 0 ){
    $sql .= " WHERE ".implode( " and ", $where);
}

echo $sql.PHP_EOL;
print_r($data);

The count($where) > 0 part will only add the where clause if there is something to add, and the implode adds the appropriate and bits as the glue.

Which with the test data gives...

SELECT * FROM `pickups` WHERE `name` like ?  and `address` like ? 
Array
(
    [0] => %name1%
    [1] => %add%
)

This only really works with strings, but you could add specific code for other data fields and add the clause into the $where array before getting to the final part which builds the statement.

Upvotes: 1

Related Questions