PHP PDO search & pagination

Question

I know that OOP is ubiquitous nowadays. Nonetheless I still write procedural PHP. It now seems to me that I had fallen into a spaghetti code trap, because my code starts to seem meaningless even to me (it's author :)

Can you please tell me how bad is this?

<?php
/**
 * Created by PhpStorm.
 * User: Taras
 * Date: 23.01.2019
 * Time: 20:01
 */
include "db_conn.php";
$currentPage = "list";
include "header.php";
// Quantity of results per page
$limit = 10;
// Check if page has been clicked
if (!isset($_GET['page'])) {
 $page = 1;
} else{
 $page = $_GET['page'];
}
// Initial offset, if page number wasn't specified than start from the very beginning
$starting_limit = ($page-1)*$limit;
// Check if search form has been submitted
if (isset($_GET['search'])) {
 $searchTerm = $_GET['search'];
 $sql = "SELECT company.id, company.name, inn, ceo, city, phone
 FROM company
 LEFT JOIN address ON company.id = address.company_id
 LEFT JOIN contact ON company.id = contact.company_id
 WHERE
 MATCH (company.name, inn, ceo) AGAINST (:searchTerm)
 OR MATCH (city, street) AGAINST (:searchTerm)
 OR MATCH(contact.name, phone, email) AGAINST (:searchTerm)";
 $stmt = $pdo->prepare($sql);
 $stmt->execute(array(':searchTerm' => $searchTerm));
 $total_results_without_limit = $stmt->rowCount();
 $total_pages = ceil($total_results_without_limit/$limit);
 $sql = "SELECT company.id, company.name, inn, ceo, city, phone
 FROM company
 LEFT JOIN address ON company.id = address.company_id
 LEFT JOIN contact ON company.id = contact.company_id
 WHERE
 MATCH (company.name, inn, ceo) AGAINST (:searchTerm)
 OR MATCH (city, street) AGAINST (:searchTerm)
 OR MATCH(contact.name, phone, email) AGAINST (:searchTerm)
 ORDER BY id DESC LIMIT $starting_limit, $limit";
 $stmt = $pdo->prepare($sql);
 $stmt->execute(array(':searchTerm' => $searchTerm));
 // Count number of rows to make proper number of pages
} else { // Basically else clause is similar to the search block except no search is being made
 $sql = "SELECT * FROM company";
 $stmt = $pdo->prepare($sql);
 $stmt->execute();
 // Again count number of rows
 $total_results = $stmt->rowCount();
 $total_pages = ceil($total_results/$limit);
 // And then make a query
 $stmt = $pdo->prepare("
 SELECT company.id, company.name, company.inn,
 company.ceo, address.city, contact.phone
 FROM company
 LEFT JOIN address
 ON company.id = address.company_id
 LEFT JOIN contact
 ON company.id = contact.company_id
 ORDER BY id ASC LIMIT $starting_limit, $limit");
 $stmt->execute();
}
?>
<main class="container">
 <section class="section-search">
 <div class="row col-auto">
 <h2>Найти компанию</h2>
 </div>
 <form action="list.php" method="get">
 <div class="row">
 <div class="form-group col-lg-6 col-md-6 col-sm-12 row">
 <label class="col-sm-4 col-form-label" for="searchTerm">
 Поиск
 </label>
 <input type="text" class="form-control col-sm-8"
 id="companyTerm" name="search">
 </div>
 <div class="float-right">
 <input type="submit" class="btn btn-primary mb-2" value="Поиск">
 </div>
 </div>
 </form>
 </section>
 <section class="section-companies">
 <table class="table table-sm">
 <thead>
 <tr>
 <th scope="col">ИНН</th>
 <th scope="col">Название</th>
 <th scope="col">Генеральный директор</th>
 <th scope="col">Город</th>
 <th scope="col">Контактный телефон</th>
 </tr>
 </thead>
 <tbody>
 <?php
 // Filling the result table with results
 while ($row = $stmt->fetch(PDO::FETCH_ASSOC)) {
 echo "<tr><td scope='row'>" . $row['inn'] . "</td>";
 echo "<td><a href='company.php?id=" . $row["id"] . "'>" . $row['name'] . "</a>" . "</td>";
 echo "<td>" . $row['ceo'] . "</td>";
 echo "<td>" . $row['city'] . "</td>";
 echo "<td>" . $row['phone'] . "</td></tr>";
 }
 ?>
 </tbody>
 </table>
 </section>
 <?php
 // Paginating part itself
 for ($page=1; $page <= $total_pages ; $page++):?>
 <a href='<?php
 if (isset($searchTerm)) {
 echo "list.php?search=$searchTerm&page=$page";
 } else {
 echo "list.php?page=$page";
 } ?>' class="links"><?php echo $page; ?>
 </a>
 <?php endfor; ?>
</main>

• greybeard: (Welcome to CodeReview!) Is your main concern with the readability of your code or that there should be a more succinct way to achieve basically the same effect?

Answer 1

Code duplication

To be honest - yes, this code is rather messy, mostly due to the code duplication. At least you could've shortened it by reusing the existing SQL, like

$sql = "SELECT company.id, company.name, inn, ceo, city, phone
 FROM company
 LEFT JOIN address ON company.id = address.company_id
 LEFT JOIN contact ON company.id = contact.company_id
 WHERE
 MATCH (company.name, inn, ceo) AGAINST (:searchTerm)
 OR MATCH (city, street) AGAINST (:searchTerm)
 OR MATCH(contact.name, phone, email) AGAINST (:searchTerm)";
$stmt = $pdo->prepare($sql);
// ...
$sql .= " ORDER BY id DESC LIMIT $starting_limit, $limit";
$stmt = $pdo->prepare($sql);

Performance problems

But the biggest problem with this code is not its structure but its ability to kill your server. Getting the number of rows returned by SELECT query is the most misused functinality in PHP.. Aside of the fact it is most of time just pointless, in a case like yours it could be dangerously harmful. Let's take your code, that looks quite simple and innocent:

$sql = "SELECT * FROM company";
$stmt = $pdo->prepare($sql);
$stmt->execute();
$total_results = $stmt->rowCount();

What does this code do? It is merely selecting all rows from the table and sending them to PHP, so it will be able to count them. Imagine there will be 100000 rows, with 1k data in each. It will result in sending 1G of data from database server to PHP. Not only it will consume a lot of resources in the process but most likely it will overflow the memory limit in PHP and make it die with the fatal error.

So you should never ever use rowCount() for pagination. Instead, you must ask a database to get you the number of rows.

Query builder

In order to make this code more maintainable (so every query part would be written only once), we must split the SQL into distinct parts and then use them to construct the particular query:

$select_count = "SELECT count(*) FROM company";
$select_data = "SELECT company.id, company.name, inn, ceo, city, phone
 FROM company
 LEFT JOIN address ON company.id = address.company_id
 LEFT JOIN contact ON company.id = contact.company_id";
$where = "WHERE MATCH (company.name, inn, ceo) AGAINST (:searchTerm)
 OR MATCH (city, street) AGAINST (:searchTerm)
 OR MATCH(contact.name, phone, email) AGAINST (:searchTerm)";
$order_limit = "ORDER BY id DESC LIMIT $starting_limit, $limit";
if (isset($_GET['search'])) {
 $parameters = [':searchTerm' => $_GET['search']];
 $sql_count = "$select_count $where";
 $sql_data = "$select_data $where $order_limit";
} else {
 $parameters = [];
 $sql_count = "$select_count";
 $sql_data = "$select_data $order_limit";
}
$stmt = $pdo->prepare($sql_count);
$stmt->execute($parameters);
$num_results = $stmt->fetchColumn();
$total_pages = ceil($num_results/$limit);
$stmt = $pdo->prepare($sql_data);
$stmt->execute($parameters);
$results = $stmt->fetchAll();

Better separation between HTML and database stuff

The rest is your code is Okay, I would only remove any mention of a database stuff from HTML. So I would rather get all the rows into array first and than just iterate over it in HTML.

 <tbody>
 <?php foreach ($results as $row): ?>
 <tr>
 <td scope='row'><?=$row['inn']?></td>
 <td><a href="company.php?id=<?=$row["id"]?>"><?=$row['name']?></a></td>
 <td><?=$row['ceo']?></td>
 <td><?=$row['city']?></td>
 <td><?=$row['phone']?></td>
 </tr>
 <?php endforeach ?>
 </tbody>

• tnsaturday: Great many thanks! You've discovered exactly what I was in doubt of, just couldn't make it clear. Can I ask you to point me at some books/online tutorials that cover topics like code reusage, proper architecture and the web app architecture as well? Because my other code is pretty simple to this :(