Oh, I didn’t realise these were from two different files. It’s not at all clear in your post #8 that the PHP in the second code segment isn’t all one big file. I notice that you separately include “result.html.php”, but that doesn’t count as a separate file as it’s merged into the main file at run-time and any variable names would continue from one to the other - which you rely upon in order to display the results. But both the code snippets I quoted are from the second code segment in your post, which I thought was all one file. If it is not, you should show clearly where the break is, and how you share the variables from the first file to the second.
Looking at the code, I see this (in the added comments)
<?php
if(isset($_POST['submit'])){
$fname=$_POST['firstname']; // ** here, you create $fname as a plain string
$lname=$_POST['lastname']; // ** and here you create $lname as a plain string
try
{
$pdo = new PDO('mysql:host=localhost;dbname=employees', 'phpuser', 'p@$$w0rd');
$pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
$pdo->exec('SET NAMES "utf8"');
}
catch (PDOException $e)
{
$error = 'Unable to connect to the database server.';
include 'error_pages/error.html.php';
exit();
}
try
{
// below, you use those plain strings in your query
$sql = "SELECT first_name, last_name FROM employees WHERE first_name like '%" . $fname . "%' and last_name like '%" . $lname . "%'";
$result = $pdo->query($sql);
}
catch (PDOException $e)
{
$error = 'Error fetching employee names: ' . $e->getMessage();
include 'error_pages/error.html.php';
exit();
}
while ($row = $result->fetch())
{
// here, you create two blank arrays, using the same names you used above
$fname = array();
$lname = array();
$fname[] = $row['first_name']; // and here, you assign values to those blank arrays.
$lname[] = $row['last_name'];
}}
include 'result.html.php';
?>
What I believe you want to do is this:
$fnames = array();
$lnames = array();
while ($row = $result->fetch())
{
$fnames[] = $row['first_name'];
$lnames[] = $row['last_name'];
}}
I’ve moved the array creation to before the loop, so it only gets created once, before the loop starts. Every time you run through the loop, you then add an element to each array for each result that comes from your query. I’ve changed the variable names as well, to avoid confusion. I’ve just added an “s” on the end of the name, because it makes sense to me that as there will be more than one value, the name reflects that.
To clarify, the only problem in this particular section was that you define the array inside the loop. This means that every time you retrieve the next result, you define the array (which also empties the array) and then add an element to it. So of course every time you retrieve a result, you throw away the previous ones. Moving the array definition to before the loop fixes this.
Moving on to the separately included code:
<h1>Here are all the employees in the database that match your selection criteria: <br></h1>
<p><?php
$fnames=array(); // what does this achieve?
foreach ($fname as $fnames) {
echo "$fnames";
}
$lnames=array(); // or this?
foreach ($lname as $lnames) {
echo "$lnames";
}?></p>
There’s no point doing those array definitions at this point. (Actually if you use the code change I made above, it will cause you a big problem because they match the names I used.) But you’re achieving nothing by creating an array at that point. The very next line in each case is a foreach loop that iterates through your results array and uses that same array that you just created, but as a plain string, which is only fine because the foreach will overwrite the array you created. So there’s no harm, but there’s also no point creating the array.
If the aim of the code is to run a query, get all the results into an array and then display them, I’d probably do something like this instead:
<?php
if(isset($_POST['submit'])){
$fname=$_POST['firstname'];
$lname=$_POST['lastname'];
try
{
$pdo = new PDO('mysql:host=localhost;dbname=employees', 'phpuser', 'p@$$w0rd');
$pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
$pdo->exec('SET NAMES "utf8"');
}
catch (PDOException $e)
{
$error = 'Unable to connect to the database server.';
include 'error_pages/error.html.php';
exit();
}
try
{
// below, you use those plain strings in your query
$sql = "SELECT first_name, last_name FROM employees WHERE first_name like :firstname and last_name like :lastname";
$prep = $pdo->prepare($sql); // use a prepared statement here
$prep->bindParam(":firstname", "%" . $fname . "%"); // first name
$prep->bindParam(":lastname", "%" . $lname . "%"); // last name
$result = $prep-execute();
}
catch (PDOException $e)
{
$error = 'Error fetching employee names: ' . $e->getMessage();
include 'error_pages/error.html.php';
exit();
}
// Get the results
if ($result) {
$results = $prep->fetchAll();
}
else {
echo "There was a problem executing the query";
exit();
} // obviously you'll do something better here.
}
include 'result.html.php';
?>
and in your display
<h1>Here are all the employees in the database that match your selection criteria: <br></h1>
<p><?php
foreach ($results as $employee) {
echo $employee['first_name'] . " " . $employee['last_name'] . "\n";
}
?>
</p>
There are things to watch out for. If your result set is going to be massive, using fetchAll() might be a problem, you might need to loop through the results (as you were originally doing) but instead of storing them in an array, display each one as it is found. That might prove to be an easier approach anyway, but as all your loop does is retrieve every result and stick it in an array, you might as well use the built in fetchAll() to do it for you.
Also - I don’t whether all those try-catch blocks are the best way to deal with any errors. I know there’s been a thread recently about PDO error trapping. I’ve left them in so as to not change too much. But basically I’ve changed:
- Use a prepared statement. This will help with the sanitising of user input
- Use fetchAll() to get all the results into a single array
You also might want to consider folding things to upper case for the search, or lower case, for consistency. If the database stores “Hugh Cornwell” you don’t want to be throwing “not found” errors (or in this case just displaying an empty list) because your user typed in “UG ORN”. You should also look at how many results come back before displaying them - use rowCount() to decide whether you should say “none found” or your current display.
