Struggling with maintainability on this MySQLi/PHP code segment

Question

I have a piece of code that works, however it's pretty messy. The objective is to retrieve all the relevant rows from a user search. For example: If the user searches 'Jo', it will retrieve John and Josh:

The code that works:

if($_SERVER["REQUEST_METHOD"] == "POST") {
  $search = trim(filter_input(INPUT_POST,"user_search",FILTER_SANITIZE_SPECIAL_CHARS));
  $search = preg_replace("#[^0-9a-z]#i", "", $search);

  if(!isset($error_message)) {
    $query = $db->query("SELECT * FROM User WHERE U_Forename LIKE '%$search%'
                    OR U_Surname LIKE '%$search%' OR U_Team LIKE '%$search'
                    OR U_City LIKE '%$search%'");
  }
}

  <?php
          if ($query->num_rows == 0) {
            $output = "No results found!";
          } else {
            echo "<h2>Search results for: " . $search . "</h2>";
            while($row = $query->fetch_assoc()) {
              $id = $row['U_ID'];
              $firstname = $row['U_Forename'];
              $lastname = $row['U_Surname'];
              $team = $row['U_Team'];
              $city = $row['U_City'];
              echo "<div class='searchresults'>";
              echo "<p> Name: " . ucfirst($firstname) . " " . ucfirst($lastname) . "</p>";
               echo "<p> Favourite Team: " . ucfirst($team) . "</p>";
              echo "<p> Location: " . ucfirst($city) . "</p>";
              echo "<a class='lift' href='profile.php?=[$id]'>View Profile</a>";
              echo "<a class='lift' href='#'>Follow Driver</a>";
              echo "</div>";
            }
          }
          ?>

What I have tried:

function getSearchResults($search) {
  global $db;
  $searchArray = array();
  $query = $db->query("SELECT * FROM User WHERE U_Forename LIKE '%$search%'
                  OR U_Surname LIKE '%$search%' OR U_Team LIKE '%$search'
                  OR U_City LIKE '%$search%'");
  if ($query->num_rows == 0) {
    echo "No results found!";
  } else {
    while($row = $query->fetch_assoc()) {
      $searchArray['U_ID'] = $row['U_ID'];
      $searchArray['U_Forename'] = $row['U_Forename'];
      $searchArray['U_Surname'] = $row['U_Surname'];
      $searchArray['U_Team'] = $row['U_Team'];
      $searchArray['U_City'] = $row['U_City'];
    }
  }
  return $searchArray;
}

However this will not work as only the first result from the fetch_assoc() is stored in the $searchArray and nothing else! Does anyone have any suggestions? I am unsure on where to begin coding next.

James.

**WARNING**: When using `mysqli` you should be using parameterized queries and [`bind_param`](http://php.net/manual/en/mysqli-stmt.bind-param.php) to add user data to your query. **DO NOT** use manual escaping and string interpolation or concatenation to accomplish this because you will create severe [SQL injection bugs](http://bobby-tables.com/). Scrubbing your string may keep your data clean but it doesn't always prevent SQL injection problems, you might forget to properly scrub something that's later injected. Using placeholders makes mistakes obvious. — tadman, Apr 11 '16 at 21:18

score 3 · Accepted Answer · answered Apr 11 '16 at 21:19

You're overwriting the array every time you run the loop. You need to create an array of arrays so you can return one array. Below, I simplified your code to append the current row to the array you're returning. You can then do a foreach on the returned data and display your code

while($row = $query->fetch_assoc()) {
  $searchArray[] = $row;
}

Also, a protip: pass your DB connector as a parameter and avoid using it as a global (which is another best practice)

function getSearchResults($search) {
   global $db;

Becomes

function getSearchResults($search, $db) {

And last but not least, don't have your function echo things (have the layer that called this function do that). Instead, just return an empty array

if ($query->num_rows == 0) {
   return $searchArray;
}

Hi @Machavity thank you for your detailed answer! How would I go about echoing the values to the screen using foreach? I am unsure on how to approach this, too. I have called the getSearchResults() function in another file and stored the returned array in a variable called $searchResult. Thanks again for your help. — James Barrett, Apr 12 '16 at 11:54
@JamesBarrett Check out [this question](http://stackoverflow.com/questions/10057671/how-does-foreach-actually-work) for details on how `foreach` works — Machavity, Apr 12 '16 at 12:13

Struggling with maintainability on this MySQLi/PHP code segment

1 Answers1