patternphpMinor
Prepared PHP statement to fetch posts for some category
Viewed 0 times
statementphpfetchpreparedforsomepostscategory
Problem
I am slowly learning PHP and have been using prepared statements.
I have a simple category.php page, which takes a category tag to display posts from that category, such as localhost/php/category.php?tag=sysadmin. I want to select the Category Title from the "j_oracle_cat" table based on the value of the incoming tag in the querystring.
My code seems to be extremely cumbersome and long-winded (I'm using MySQLi instead of PDO):
I just wondered if I am making a meal of this, and if the code is over the top, or if I am on the right track (both with the prepared statement and handling the incoming querystring variable).
I have a simple category.php page, which takes a category tag to display posts from that category, such as localhost/php/category.php?tag=sysadmin. I want to select the Category Title from the "j_oracle_cat" table based on the value of the incoming tag in the querystring.
My code seems to be extremely cumbersome and long-winded (I'm using MySQLi instead of PDO):
0) { Header("Location: default.php"); }
/* sql */
$cat_title_sql = "SELECT fld_title FROM j_oracle_cat WHERE fld_tag = ? LIMIT 1";
/* category variable */
$foo_cat = $foo;
/* initialise and prepare */
$stmt = $conn->stmt_init();
$result_cat_title = $stmt->prepare($cat_title_sql);
if(!$result_cat_title) { trigger_error("Error preparing statement: $stmt->errorSQL query: $cat_title_sql", E_USER_ERROR); }
/* bind */
$result = $stmt->bind_param('s',$foo_cat);
if(!$result) { trigger_error("Error binding parameter: $stmt->error", E_USER_ERROR); }
/* execute */
$stmt->execute();
/* store */
$stmt->store_result();
$count_cat = $stmt->num_rows;
/* if no results go to homepage */
if ($count_cat == 0) {
Header("Location: ../default.php");
} elseif ($result) {
$stmt->bind_result($fld_title);
while ($stmt->fetch()) {
$cat_ttl = $fld_title;
}
}
/* ... header include down here */
?>
I just wondered if I am making a meal of this, and if the code is over the top, or if I am on the right track (both with the prepared statement and handling the incoming querystring variable).
Solution
$foo2 = strpos($foo, '\'');
if ($foo2 > 0) { Header("Location: default.php"); }- Creating another variable for this, especially with a generic name like "foo2", is superfluous and should be avoided.
if (strpos(..) > 0)will do just fine.
- Just redirecting with a
headerdoes not stop the script execution. Your script will continue to run, making this rather pointless. You need toexitexplicitly.
- This check will fail if the quote is the first character, since
strposreturns0in that case.
- Why are you worrying about quotes in particular at all? There's really no point to this.
What you may want to check for instead is whether the value is defined at all.
$foo_cat = $foo;There's no point in reassigning this variable yet again.
$result_cat_title = $stmt->prepare($cat_title_sql);
if(!$result_cat_title) ...Using yet another rather mis-named variable is pretty superfluous, again.
trigger_error(..)If you're using a (poorly) user-defined error handler (or somebody who may be using this code in the future adds one), your script may not actually exit when you trigger an error. You should either still explicitly
exit after this, or perhaps switch to exceptions.$count_cat = $stmt->num_rows;Again a rather superfluous variable assignment.
Header("Location: ../default.php");Again a header which does not terminate script execution.
You should also consider indicating a 404 Not Found error instead of a silent redirect.
else if ($result)Why are you checking
$result here? That was a variable used in completely different context. Also, since the previous header statement was supposed to terminate the script, you can skip this check entirely.while ($stmt->fetch())You're only expecting a single result, no need for a loop.
$cat_ttl = $fld_title;Yet another superfluous variable assignment.
You should take care of HTML injection/syntax issues and HTML-encode your output appropriately.
Beyond this indentation and variables names are often sub-optimal.
In summary:
stmt_init();
$query = 'SELECT ld_title FROM j_oracle_cat WHERE ld_tag = ? LIMIT 1';
if (!$stmt->prepare($query)) {
throw new Exception("Error preparing statement: $stmt->error, SQL query: $query");
}
if (!$stmt->bind_param('s', $_GET['tag'])) {
throw new Exception("Error binding parameter: $stmt->error");
}
$stmt->execute();
$stmt->store_result();
if ($stmt->num_rows == 0) {
header('HTTP/1.0 404 Not Found');
exit;
}
$stmt->bind_result($title);
$stmt->fetch();
?>
Code Snippets
$foo2 = strpos($foo, '\'');
if ($foo2 > 0) { Header("Location: default.php"); }$foo_cat = $foo;$result_cat_title = $stmt->prepare($cat_title_sql);
if(!$result_cat_title) ...trigger_error(..)$count_cat = $stmt->num_rows;Context
StackExchange Code Review Q#120051, answer score: 3
Revisions (0)
No revisions yet.