2
\$\begingroup\$

This is my first routing system on PHP. As you can see it's very simple. Any advice or tip about it that can make it better? I want to make it better so I could use it in real websites.

First I include db connect, then get the URL, if the URL have special index like post id or post name, then include post.php or if $sp_link[4] == profile then load the profile.php.

<?php
    require_once 'inc/header.php';
    include_once 'inc/nav.php';


    $actual_link = (isset($_SERVER['HTTPS']) ? "https" : "http") . "://$_SERVER[HTTP_HOST]$_SERVER[REQUEST_URI]";
    // echo $actual_link;
    $sp_link = explode("/", $actual_link);
    print_r($sp_link);


    $sql ='SELECT * from posts WHERE post_id=? AND post_name=?';
    $stmt=$pdo->prepare($sql);
    $stmt->execute([@$sp_link[5], @$sp_link[6]]);
    $post=$stmt->fetch();

    if ($sp_link[4] == 'blog' && @$sp_link[5] == '' ) {
        // @include 'blog.php';
        //$page_title = $post->post_title;
    }
    else if ($sp_link[4] == 'profile' && @$sp_link[5] == '' ) {
        @include_once 'profile.php';
    }
    else if ($sp_link[4] == 'login' && @$sp_link[5] == '' ) {
        @include 'login.php';
        $page_title = 'ورود';
    }
    else if ($sp_link[4] == 'signup' && @$sp_link[5] == '' ) {
        @include 'signup.php';
        $page_title = 'ورود';
    }
    else if ($sp_link[4] == 'blog' && @$sp_link[5] == $post->post_id && @$sp_link[6] ==     $post->post_name) {
        @include 'post.php';
        $page_title = $post->post_title;

    }
    else if (($sp_link[4] == '' || $sp_link[4] == 'index' || $sp_link[4] == 'index.php') &&     @$sp_link[5] == '') {
           $page_title = 'سکان آکادمی | آموزش برنامه نویسی';
            include 'home.php';
    }
    else if (($sp_link[4] == 'blog' && $sp_link[5] == '12342') && @$sp_link[6] == '') {
        include_once 'post.php';
    } else {
        include_once '404.php';
    }
\$\endgroup\$
1
\$\begingroup\$

Firstly, I would consider switching off error suppression, and add in Exception handling. You could in this example add in a condition to check if each file exists before including it, for instance:

if ($sp_link[4] == 'profile' && @$sp_link[5] == '' && file_exists('profile.php')) {
    include_once 'profile.php';
}

It looks like you're using the query only to return a page title, and only in one instance. Is this correct? If so, amend you query to only return the data that you want, rather than returning the whole dataset from any particular table, for instance, if you only want the title tuple, change it to this:

$sql ='SELECT `title` FROM `posts` WHERE `post_id`=? AND `post_name`=?';
$stmt=$pdo->prepare($sql);
$stmt->execute([$sp_link[5], $sp_link[6]]);
$post=$stmt->fetch(PDO::FETCH_ASSOC);

To improve this further, you could turn this block of PHP into a function or method. Here's how one might achieve this aim:

function simpleSelect($fields = '*', $table = 'posts', $postId = '', $postName = '', $stmt = null){
    if(is_array($fields)){
        $fields = implode(',', $fields);
    }
    $sql = "SELECT `{$fileds}` FROM `{$table}` WHERE `post_id`=? AND `post_name`=?";
    $stmt = $pdo->prepare($sql);
    $stmt->execute([$postId, $postName]);
    return $stmt->fetch(PDO::FETCH_ASSOC);
}

Now the top of your code starting from $sql ='SELECT * from posts WHERE post_id=? AND post_name=?'; and ending with $post=$stmt->fetch(); becomes this:

$post = simpleSelect('title', 'posts', $sp_link[5], $sp_link[6], $stmt);

If you need to select multiple fields from the posts table, then you could do this (imagining that you want title and name fields returned):

$post = simpleSelect(['title', 'name'], 'posts', $sp_link[5], $sp_link[6], $stmt);

Finally, consider moving to an object-orientated approach. Exception handling and OO programming will be covered elsewhere.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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