2
\$\begingroup\$

For common users I have 10 queries and for registered users I have 30 queries like this one below, only to print out on the screen the link for the querystrings, a guy said to me that I should escape the values of the menus that use querystrings, I'm having a problem with google insights, my TTFB is at 0,89s on Desktop and 0,24s on Mobile.

This code below is like a "validator". When I create a page I need to insert on my table the title, slug_link and the entry_type that can be item or pagina, otherwise I'm not able to open the page on the browser. It's good. I think I got a bit of security, and this is not a problem.

I'm thinking... maybe I can insert on my table the name/slug_link of my pages and use a variable to the links? Example: $slugHome = ?p=home and on the HTML I would do <a href="?p=<?php echo htmlentities($slugHome, \ENT_QUOTES, 'UTF-8', false); ?>">Home</a>. This way I don't need to do requests to my database for each item of the menu. I'm not a expert in security, so I want to know what you guys think.

How can I print on the screen the menus without making so many queries?

This is the "validator" code, I also use this global variables. You will see the queries on the other snippet below:

$isItems = !empty($_GET['page']);
$slug =  'home';

if ($isItems) {
    $slug = $_GET['page'];
} elseif (!empty($_GET['p'])) {
    $slug = $_GET['p'];
}

try {
    $stmt = $conn->prepare('SELECT `id`, `title`, `dropmenuGenero`, `epTitle`, `itemName`, `letra`, `data`, `datePublished`, `descricao`, `capa`, `epCapa`, `alt`, `audio`, `qualidade`, `tipo`, `epNum`, `keywords`, `item`, `dateModified`, `slug_url`, `slugForitemPage`, `slug_link`, `entry_type`, `formato`, `status` FROM `table_item` WHERE `slug_link` = :slug_link AND `entry_type` = :entry_type');
    $stmt->execute([
        ':entry_type' => $isItems ? 'item' : 'pagina', 
        ':slug_link'  => $slug
    ]);
    if (!$NF = $stmt->fetch(\PDO::FETCH_ASSOC)) {

        throw new \InvalidArgumentException('Items title ' . htmlentities($title, \ENT_QUOTES, 'UTF-8', false) . ' not found in database');
    }

    $id                   = $NF['id'];
    $title                = $NF['title'];
    $epTitle              = $NF['epTitle'];
    $itemName            = $NF['itemName'];
    $letra                = $NF['letra'];
    $data                 = $NF['data'];
    $datePublished        = $NF['datePublished'];
    $dateModified         = $NF['dateModified'];
    $descricao            = $NF['descricao'];
    $capa                 = $NF['capa'];
    $epCapa               = $NF['epCapa'];
    $alt                  = $NF['alt'];
    $audio                = $NF['audio'];
    $qualidade            = $NF['qualidade'];
    $tipo                 = $NF['tipo'];
    $epNum                = $NF['epNum'];
    $keywords             = $NF['keywords'];
    $url                  = $NF['slug_url'];
    $dropmenuGenero       = $NF['dropmenuGenero'];
    $slug                 = $NF['slug_link'];
    $slugForitemPage     = $NF['slugForitemPage'];
    $entry_type           = $NF['entry_type'];
    $formato              = $NF['formato'];
    $status               = $NF['status'];

} catch (\InvalidArgumentException $e) {
    header('Location: ?p=home'); 
    exit;
} catch (\Exception $e) {
    header('Location: error.php?e=Algo deu errado :/');
    throw $e;
}

function sanitize($data, $filter = \FILTER_SANITIZE_STRING) {
    if ($data = filter_var(trim($data), $filter)) {
        $data = preg_replace('/http(s)?:\/\//', '', $data);
    }   
    return $data;
}

$loadPage = null;

if ($sanitizedName = sanitize($isItem ? $title : $slug)) {
    $loadPageSuffix = ($isItem ? '/items/' : '/page_');
    $loadPage =  __DIR__ . $loadPageSuffix . $sanitizedName . '.php';
}
if (null === $loadPage || !is_file($loadPage)) {
    header('HTTP/1.1 404 Not Found'); 
    exit;
}

This is 3 of 10-30 queries that I use only to print on the screen a list of menus:

<?php
titleHome = 'Página Inicial';
$pageHome = $conn->prepare("SELECT `title`, `slug_link` FROM `table_tudo` WHERE `title` = :title");
$pageHome->bindParam(':title', $titleHome, PDO::PARAM_STR);
$pageHome->execute();
?>

<?php foreach($pageHome as list($pageTitle, $pageSlug)) { ?>
  <li class="nav-item pr-2 navbarItem">
    <a class="nav-link" href="?p=<?php echo htmlentities($pageSlug, \ENT_QUOTES, 'UTF-8', false); ?>"><?php echo htmlentities($pageTitle, \ENT_QUOTES, 'UTF-8', false); ?></a>
  </li>
<?php } ?>

<?php
query for dropdows-menus:
$titleListaDropDown = 'Lista de Items';
$pageListDropDown = $conn->prepare("SELECT `title`, `slug_link` FROM `table_tudo` WHERE `title` = :title");
$pageListDropDown->bindParam(':title', $titleListaDropDown, PDO::PARAM_STR);
$pageListDropDown->execute();

$entry_typePageList = 'pagina';
$pageList = $conn->prepare("SELECT `dropmenuList`, `slug_link` FROM `table_tudo` WHERE `entry_type` = :entry_type AND `dropmenuList` IS NOT NULL");
$pageList->bindParam(':entry_type', $entry_typePageList, PDO::PARAM_STR);
$pageList->execute();
?>

<?php foreach($pageListDropDown as list($pageTitleLDD, $pageSlugLDD)) { ?>
<li class="nav-item dropdown pr-2 navbarItem ">
  <a class="nav-link dropdown-toggle" href="#" id="navbarDropdownMenuLink" role="button" data-toggle="dropdown" aria-haspopup="true" aria-expanded="false">
    <?php echo htmlentities($pageTitleLDD, \ENT_QUOTES, 'UTF-8', false); ?>
  </a>
  <div class="dropdown-menu" aria-labelledby="navbarDropdownMenuLink">
    <?php foreach ($pageList as list($pageTitleL, $pageSlugL)): ?>
      <a class="dropdown-item" href="?p=<?php echo htmlentities($pageSlugL, \ENT_QUOTES, 'UTF-8', false); ?>"><?php echo htmlentities($pageTitleL, \ENT_QUOTES, 'UTF-8', false); ?></a>
    <?php endforeach; ?>
  </div>
</li>
<?php } ?>
\$\endgroup\$
2
\$\begingroup\$

Looking at most of the queries, they appear to be almost the same, except for the values of some parameters. There is nothing wrong with having multiple queries to the database, but you can eliminate the repetitive queries by creating functions or classes for them.

I see two variations of the same basic query:

  • Find by title
  • Find by entry_type

A sample class:

class MenuRepository {
    private $conn;

    public function __construct($conn) {
        $this->conn = $conn;
    }

    public function findByTitle($title) {
        $query = $this->conn->prepare("SELECT `title`, `slug_link` FROM `table_tudo` WHERE `title` = :title");
        $query->bindParam(':title', $title, PDO::PARAM_STR);
        $query->execute();

        return $query;
    }

    public function findByEntryType($entryType) {
        $query = $this->conn->prepare("SELECT `dropmenuList`, `slug_link` FROM `table_tudo` WHERE `entry_type` = :entry_type AND `dropmenuList` IS NOT NULL");
        $query->bindParam(':entry_type', $entryType, PDO::PARAM_STR);
        $query->execute();

        return $query;
    }
}

Now you eliminate the queries from your code, and just create an instance of your "menu repository" and call methods on it when needed:

So there is the abstraction you need to create. Two different functions that each have one copy of the query, and all you need to do is pass in different arguments each time:

<?php
$menuItems = new MenuRepository($conn);
$pageHome = $menuItems->findByTitle('Página Inicial');
$pageListDropDown = $menuItems->findByTitle('Lista de Items');
$pageList = $menuItems->findByEntryType('pagina');
?>

<?php foreach($pageHome as list($pageTitle, $pageSlug)) { ?>
  <li class="nav-item pr-2 navbarItem">
    <a class="nav-link" href="?p=<?php echo htmlentities($pageSlug, \ENT_QUOTES, 'UTF-8', false); ?>"><?php echo htmlentities($pageTitle, \ENT_QUOTES, 'UTF-8', false); ?></a>
  </li>
<?php } ?>

<?php foreach($pageListDropDown as list($pageTitleLDD, $pageSlugLDD)) { ?>
<li class="nav-item dropdown pr-2 navbarItem ">
  <a class="nav-link dropdown-toggle" href="#" id="navbarDropdownMenuLink" role="button" data-toggle="dropdown" aria-haspopup="true" aria-expanded="false">
    <?php echo htmlentities($pageTitleLDD, \ENT_QUOTES, 'UTF-8', false); ?>
  </a>
  <div class="dropdown-menu" aria-labelledby="navbarDropdownMenuLink">
    <?php foreach ($pageList as list($pageTitleL, $pageSlugL)): ?>
      <a class="dropdown-item" href="?p=<?php echo htmlentities($pageSlugL, \ENT_QUOTES, 'UTF-8', false); ?>"><?php echo htmlentities($pageTitleL, \ENT_QUOTES, 'UTF-8', false); ?></a>
    <?php endforeach; ?>
  </div>
</li>
<?php } ?>

It reduces the boiler plate code for each query, and helps to de-clutter the the code that renders the HTML.

There is still some repetition here if the two menu titles and the entry types are need on other pages. Then it's just a matter of parameterizing those three things.

\$\endgroup\$
  • \$\begingroup\$ I don't want to use the database to the menus, can i just set a variable like this? $home <a href="?p=<?php echo htmlentities($home, \ENT_QUOTES, 'UTF-8', false); ?>">Home</a> is it safe? \$\endgroup\$ – Susi Dec 11 '18 at 17:52

Your Answer

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

Not the answer you're looking for? Browse other questions tagged or ask your own question.