Is There A Better Way Of Managing Variables In Query Than This?

If I need to query certain data from the database depending on some variables is there a better way than doing this ?

Solution 1:

At this level, I'd say there isn't a better way other than writing a set of code that fully generates sql.. Which, from one perspective, you've effectively already started doing and from another perspective is way overkill for an "if x then this sql else that sql end" scenario like this. There's been a suggestion to parameterise values properly, but that is somewhat moot if you're in full control of the sql and not concatenating user data into it and could make thinks even more convoluted

In terms of keeping things readable and clear it's probably not worth getting too involved in making an sql that is more variable than query. There may be more utility in just writing out the query again in altered form if the choices/variations are limited (your example contains only 2 choices)

Solution 2:

As commented by several SO folks the best practice in every web app is to used parameterized statements. This protects against SQL injection, and improves performance.

The specificity of your use case is that :

• The comparison operator cannot be passed as a parameter (I can’t think of any RDBMS that would allow that)

• You are not passing POSTed values directly to the query. Instead you are using them to decide which values should be used in the query. This means that you are NOT exposing yourself to SQL injection

As a conclusion, your approach look OK (provided that you do use a parameter for the « $number » parameter instead of passing it directly to the query - you said you do it, but didn’t show that part of the code).

When the query gets more complex, you want to stick to the above principles (use parameters whenever possible).

If complexity becomes hard to manage, then you might consider moving to a ORM, an Object Relational Mapper, that creates another level of indirection between your code and raw SQL, hence allowing you to manage more complicated requirements without worrying too much about actual SQL. There are many solutions available in PHP such as Doctrine, propel, ...

Solution 3:

Your query is safe, so ignore all the banter about injection attacks.

If you expect your battery of conditions to continue growing, then I recommend a lookup array instead of if-elseif-else or case-switch because they end up bloating your script. Consider somthing like this...

$conditions = [
    1 =>"< 3",
    2 =>"> 10",
];

if (empty($_POST['condition']) || !isset($conditions[$_POST['condition']])) {
    //write default behavior
} else {
    //use $conditions[$_POST['condition']] in your query
}

This data structure and process makes extending the battery of conditions clean, concise, and efficient.

Solution 4:

Yes, there is a better way. Besides using prepared statements to prevent SQL injection, you can use the same data to generate the selection in the frontend as well as building the query. Look at this function for example:

public function getFilterOptions(): array
{
    return [
        ['name' => 'Age >30', 'comparator' => '>', 'comparable' => 30],
        ['name' => 'Age until 21', 'comparator' => '<=', 'comparable' => 21],
    ];
}

You can then use this method to generate the filter selection in the frontend:

echo'<select name="query_filter">';
foreach (getFilterOptions() as$key => $option) {
    echo'<option value="' . $key . '">' . $option['name'] . '</option>';
}
echo'</select>';

Please bear with me for this code as I don't know what templating system the questioner is using, it made sense to use plain PHP for an example.

And in your filter code, you can directly access the filter options with the given index:

$optionIndex = $_POST['query_filter'];
// TODO: you should check first if this index exists and handle errors appropriately$option = getFilterOptions()[$optionIndex];

// building the query will be easy then...// note: it is not possible to dynamically bind operators of a query$stmt = $pdo->prepare("SELECT * FROM persons WHERE age $option['comparator'] :comparable");
$stmt->execute($option);
// or explicit: $stmt->execute(['comparable' => $option['comparable']]);// using PDO is actually not really necessary here as the options are hard coded// and not user-given, but it is best practice anyway...

The advantage of doing it this way is that you have all your filter options in a central place, making changes both easier and safer. Also the code of selecting the filters based on the user input is a lot less than if you use multiple if and else if statements.

Of course this is only a very basic example, you can improve on it quite a lot, especially with more complex filters. You could for example also have the database column be part of your filter options. But I just wanted to give you a hint as to what would be possible.

Share