SQL INJECTION
^ Please Google that! Your code is seriously vulnerable! Your data can be stolen or deleted...
You have to sanitize your inputs at least with mysqli_real_escape_string()
Even better would be to take proper countermeasures to SQL injection and use prepared statements and parametrized queries! (as shown in the code below)
I think the best approach would be to handle the logic by altering the query based on the values of the variables:
$sql = "SELECT p.price,
p.type,
p.area,
p.floor,
p.construction,
p.id as propertyID,
CONCAT(u.name, ' ',u.family) as bname,
p.type as ptype,
n.name as neighborhoodName,
CONCAT(o.name,' ',o.surname,' ',o.family) as fullName
FROM `property` p
LEFT JOIN `neighbour` n ON p.neighbour = n.id
RIGHT JOIN `owners` o ON p.owner = o.id
LEFT JOIN users u ON p.broker = u.id
WHERE `neighbour`= :current "; //note: ending white space is recommended
//lower boundary clause -- if variable null - no restriction
if(!is_null($priceFrom){
sql = sql . " AND `price` >= :priceFrom "; // note: whitespace at end and beginning recommended
}
//upper boundary -- better than to set it to an arbitrary "high" value
if(!is_null($priceTo)){
sql = sql . " AND `price` <= :priceTo "; // note: whitespace at end and beginning recommended
}
This approach allows for any upper value: if there is a serious inflation, a different currency, or suddenly the code will be used to sell housese and there will be products with prices > 200000, you don't need to go out and change a lot of code to make it show...
The parameters need to be bound when executing the query of course:
$stmt = $dbConnection->prepare(sql);
$stmt->bind_param('current', $current);
if(!is_null($priceFrom)){
$stmt->bind_param('priceFrom', $priceFrom);
}
if(!is_null($priceTo)){
$stmt->bind_param('priceTo', $priceTo);
}
//execute and process in same way
$stmt->execute();
Also note: from your code it seems you are issuing queries in a loop. That is bad practice. If the data on which you loop comes
- from the DB --> use a
JOIN
- from an array or other place of the code --> better use an
IN
clause for the elements
to fetch all data with one query. This helps a lot both in organizing and maintaining the code and results generally in better performance for the most cases.