Problem
I’m using jQueryUI’s Autocomplete widget for a product search. The data is in a database, so as the souce I passed it an AJAX request. My problem is, that I need some data about the currently focused option, so in the autocomplete’s focus method, I’m calling another AJAX request to the same PHP file, to get some other data.
Here’s the jQuery code I’m currently using:
$('.termeknev.ujtermek').autocomplete({
autoFocus: true,
minLength: 1,
source: function (request, response) {
$.ajax({
url: "/backend/termekek-query.php",
type: "POST",
data: {
"term": request.term
},
success: function (data) {
response($.map(JSON.parse(data), function (items) {
return {
value: items.nev,
label: items.nev
}
}));
},
error: function (error) {}
});
},
focus: function (event, ui) {
var $elem = event.target,
$egysegar;
$.ajax({
url: "/backend/termekek-query.php",
type: "POST",
data: {
"term": ui.item.value
},
success: function (data) {
$egysegar = data.egysegar;
$($elem).parent().parent('.ujtermek_wrapper').find('td.egysegar').text($egysegar);
}
});
}
});
And here’s the pretty simple PHP file it sends the request to:
<?php
if (isset($_SERVER['HTTP_X_REQUESTED_WITH']) && strtolower($_SERVER['HTTP_X_REQUESTED_WITH']) === 'xmlhttprequest') {
if(isset($_POST["term"]) && !empty($_POST["term"])) {
require_once "../includes/constants.php";
require '../includes/class.db.php';
$searchTerm = trim($_POST["term"]);
$db = new db(CONNECTION_STRING, DATABASE_USERNAME, DATABASE_PASSWORD);
$bind = array(
":term" => "%$searchTerm%"
);
$results = $db->select(TERMEKEK_TABLE, "nev LIKE :term", $bind, "nev, egysegar");
echo json_encode($results);
}
}
?>
Note that I’m also using a PDO wrapper class, but that’s irrelevant right now.
Anyway, is there any way to make this cleaner?
Solution
Regarding to readability, the code structure is actually great and it’s easy to read the code.
The only thing, regarding to readability, is that you have mixed quotes everywhere.
But still, your code has things you can change:
Javascript:
You are using the autocomplete plugin with a request to the server.
You can skip that whole trouble and simply use the source
option.
This will save you a lot of time.
If you don’t like it (for whatever reason it is), you should do something on the error
event on your $.post
call.
Displaying an error message is better than doing nothing.
Ultimately, you can implement a caching system, but this is arguable.
If you still don’t want to use the source
option, you should remove the var $egysegar
.
That var is completely useless.
This line:
$($elem).parent().parent('.ujtermek_wrapper').find('td.egysegar').text($egysegar);
Can be safely replaced by this one:
$($elem).parent().parent('.ujtermek_wrapper').find('td.egysegar').text(data.egysegar);
Of even shorter:
$($elem).parents('.ujtermek_wrapper').find('td.egysegar').text(data.egysegar);
PHP:
On your php file, you have this line:
if(isset($_POST["term"]) && !empty($_POST["term"])) {
According to the php documentation for the function empty()
:
Returns FALSE if var exists and has a non-empty, non-zero value. Otherwise returns TRUE.
The following things are considered to be empty:
–""
(an empty string)
–0
(0 as an integer)
–0.0
(0 as a float)
–"0" (0 as a string)
–NULL
–FALSE
–array()
(an empty array)
–$var;
(a variable declared, but without a value)
This is a terrible idea!
If the search term is "0"
, it won’t provide any results.
Replace your code with this:
if(isset($_POST["term"]) && $_POST["term"] !== "") {
This will solve the issue by verifying if the term isn’t an empty string by using a strict comparison.
Right below, you have this:
require_once "../includes/constants.php";
require '../includes/class.db.php';
You mixed the quotes and you used 2 different instructions to include files.
I assume you can safely use require
here, unless you include it on the file class.bd.php
.
This will reduce the overhead of checking if the file was included before or not.
More yet (as a sidenote): you can use include
instead! (assuming the file will always exist).
On the next line, you have this block:
$searchTerm = trim($_POST["term"]);
$db = new db(CONNECTION_STRING, DATABASE_USERNAME, DATABASE_PASSWORD);
$bind = array(
":term" => "%$searchTerm%"
);
$results = $db->select(TERMEKEK_TABLE, "nev LIKE :term", $bind, "nev, egysegar");
echo json_encode($results);
The code itself is fine, but why you store trim($_POST["term"])
in a variable to use it inside a string?
This makes readability a bit hard and it is a terrible idea on this code.
Remove that variable and concatenate it with the "%"
string.
This is the result:
$bind = array(
":term" => "%".trim($_POST["term"])."%"
);
This is the right way to do it.
Edit (2014/12/01):
Since you are using the LIKE
clause, you should escape the %
character or it will return funky results.
You should escape it.
A great suggestion can be found here: How to escape literal percent sign when NO_BACKSLASH_ESCAPES option is enabled?
Using that brilliant idea, your code would be like this:
$bind = array(
":term" => "%".str_replace('%','|%',trim($_POST["term"]))."%"
);
$results = $db->select(TERMEKEK_TABLE, "nev LIKE :term escape '|' ", $bind, "nev, egysegar");
Or similar.
Outside the scope of the review, you can replace the whole block with only 2 lines:
$db = new db(CONNECTION_STRING, DATABASE_USERNAME, DATABASE_PASSWORD);
echo json_encode( $db->select(TERMEKEK_TABLE, "nev LIKE :term escape '|' ", array( ":term" => "%".str_replace('%','|%',trim($_POST["term"]))."%" ) , "nev, egysegar") );