4

I'm trying to echo out dynamic php titles depends on page id for seo purposes.

First of all, all of my pages includes header.php

And this is how my header.php begins like:

include("database.php");

//connect tables in order to echo out titles

//1- connect to categories table if isset get category_id
if (isset($_GET["category_id"])) {
$query = $handler->query("SELECT * FROM categories WHERE category_id = ".$_GET['category_id']." ");
while($r = $query->fetch()) {
  $title = $r["title"];

  }
}

//2- connect to articles table if isset get article_id
if (isset($_GET["article_id"])) {
$query = $handler->query("SELECT * FROM articles WHERE article_id = ".$_GET['article_id']." ");
while($r = $query->fetch()) {
  $title = $r["title"];

  }
}

//3- connect to users table if isset get user_id
if (isset($_GET["user_id"])) {
$query = $handler->query("SELECT * FROM users WHERE user_id = ".$_GET['user_id']." ");
while($r = $query->fetch()) {
  $title = $r["username"];

  }
}

And after I feel free to echo out title like below:

 <title><?php if (isset($_GET["category_id"])) { echo $title; echo " |"; } 
              if (isset($_GET["article_id"])) { echo $title; echo " |"; } 
              if (isset($_GET["user_id"])) { echo $title; echo " |"; } ?> mypage.com</title>

*

This is result:

on category.php?category_id=1 Page title is: "Category 1 | mypage.com"

on article.php?article_id=1 Page title is: "Article 1 | mypage.com"

on users.php?user_id=1 Page title is: "Username 1 | mypage.com"

*

My question is, I am wondering if I over load the database with this structure?

When I'm on users.php?user_id=1 is php still execute this code?:

//1- connect to categories table if isset get category_id
if (isset($_GET["category_id"])) {
$query = $handler->query("SELECT * FROM categories WHERE category_id = ".$_GET['category_id']." ");
while($r = $query->fetch()) {
  $title = $r["title"];

  }
}

or because of I'm using if (isset($_GET["category_id"])) { on the begining I'm not connecting to database and there is no over load to database?

LetsSeo
  • 875
  • 7
  • 20
  • 2
    Queries will be executed only when the defined conditions are met. BTW your code is vulnerable to SQL injections as you are taking the user input directly. – Rehmat Dec 20 '15 at 11:41
  • thanks for warning, I was thinking to take care of security when I make sure it's okay to build such structure. So I believe it's okay. right? – LetsSeo Dec 20 '15 at 11:46
  • 1
    It seems okay but you can achieve this through another way too. I'm posting an answer. – Rehmat Dec 20 '15 at 12:43
  • 1
    LetsSeo, you should always build your code with security in mind from the start. There's nothing more tedious than going through every query in your code, and changing them - what if you miss something? As it goes, it would cost you an extra 2 lines in your code to use a prepared statement instead of a query. [mysqli prepared statements](http://www.w3schools.com/php/php_mysql_prepared_statements.asp) [PDO prepared statements](http://prash.me/php-pdo-and-prepared-statements/) – Adam Copley Dec 20 '15 at 13:09
  • additionally there's this [LINK](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php) on stackoverflow community wiki. - i recommend using this – Adam Copley Dec 20 '15 at 13:14

1 Answers1

4

It seems okay but to get rid of too many conditions, I'd recommend another way. Generate the title on each page, assign it to a variable (before including your header.php) and then simply echo out the title.

Here is an example:

category.php:

<?php
    require_once"database.php";
    if(isset($_GET["category_id"])) {
        $query = $handler->query("SELECT * FROM categories WHERE category_id = ".$_GET['category_id']." ");
        while($r = $query->fetch()) {
        $title = $r["title"];
        }
    } else {
        // redirect somewhere else
    }
    include"header.php";
?>
    <body>
        Content goes here
    </body>
</html>

header.php:

<!DOCTYPE html>
<html>
    <head>
        <title><?php echo $title;?></title>
    </head>

Do the same for other files too. i.e. your users.php file will look like this:

<?php
    require_once"database.php";
    if (isset($_GET["user_id"])) {
        $query = $handler->query("SELECT * FROM users WHERE user_id = ".$_GET['user_id']." ");
        while($r = $query->fetch()) {
            $title = $r["username"];
        }
    } else {
        // redirect somewhere else
    }
    include"header.php";
?>
    <body>
        Content goes here
    </body>
</html>

header.php file will remain the same. This will save some of your server resources for sure as the server doesn't need to look for multiple criteria and the code becomes cleaner too.

Rehmat
  • 4,681
  • 3
  • 22
  • 38