3
\$\begingroup\$

I'm working on a site where you can read about different songs, listen to them, rate them etc, same with their singer.

I've been thinking about how I could create a simple, yet perfectly working pagination script using PHP, and yesterday at night I came up with the code which you will see below.

So first this is the final result of the code below:

Desktop view

enter image description here

Mobile view

enter image description here

The function which returns the number of comments:

function countComments($ID,$table){
    $st=$this->conn->prepare("SELECT COUNT(*) FROM ".$table."_comments WHERE ".$table."ID=?");
    $st->execute([$ID]);
    return $st->fetchall();
}

Deciding which table to choose the comments from, based on which ID is set, song or singer:

if(isset($_GET['songid'])){
    $table="song";
    $ID=$_GET['songid'];
}
else{
    $table="singer";
    $ID=$_GET['singerid'];
}

Storing the number of comments in the total variable, then setting all variable equal with total:

$countComments=$comments->countComments($ID,$table);
$total;
foreach($countComments as $key){
    $total=$key[0];
}
$all=$total;

Then counting the number of pages:

$page=1;
$limit_end=15;
$limit_start=($_GET['page']*$limit_end)-$limit_end;
while($all>$limit_end){
    $page++;
    $all-=$limit_end;
}
$all_pages=$page;
$page_diff=$all_pages-$_GET['page'];

Defining the links, based on which ID is set, song or singer:

if(isset($_GET['songid'])){
    $next_link="/comments/song/".$_GET['songid']."/".($_GET['page']+1);
    $prev_link="/comments/song/".$_GET['songid']."/".($_GET['page']-1);
    $next_next_link="/comments/song/".$_GET['songid']."/".($_GET['page']+2);
    $prev_prev_link="/comments/song/".$_GET['songid']."/".($_GET['page']-2);
    $last_link="/comments/song/".$_GET['songid']."/".$page;
    $first_link="/comments/song/".$_GET['songid']."/1";
}
else{
    $next_link="/comments/singer/".$_GET['singerid']."/".($_GET['page']+1);
    $prev_link="/comments/singer/".$_GET['singerid']."/".($_GET['page']-1);
    $next_next_link="/comments/singer/".$_GET['singerid']."/".($_GET['page']+2);
    $prev_prev_link="/comments/singer/".$_GET['singerid']."/".($_GET['page']-2);
    $last_link="/comments/singer/".$_GET['singerid']."/".$page;
    $first_link="/comments/singer/".$_GET['singerid']."/1";
}

Here's the function that returns with the page buttons, using Heredoc:

function pageButtons($link,$page,$button_class,$mobile){
    return <<<HTML
    <a href="{$link}">
        <button type="button" class="btn btn-sm {$button_class} page-buttons">
            <span class="d-none d-lg-block">
                {$page}
            </span>
            <span class="d-block d-lg-none font-weight-bold">
                {$mobile}
            </span>
        </button>
    </a>
HTML;
}

And finally the code which shows the result:

<div class="text-center m-2 page-buttons-container">
            <span>
                <span class="font-weight-bold">
                    <?php
                        echo $all_pages;
                    ?>
                </span>
                <span>
                    pages
                </span>
            </span>
            <br>
<?php
if($_GET['page']>1){
    if(($_GET['page']+1)-3>1){
        echo $comments->pageButtons($first_link,"First","btn-outline-light","<i class='fas fa-angle-double-left'></i>");
    }
    if(($_GET['page']+1)-1>1){
        echo $comments->pageButtons($prev_link,"Prev","btn-outline-light","<i class='fas fa-angle-left'></i>");
    }
    if(($_GET['page']+1)-2>1){
        echo $comments->pageButtons($prev_prev_link,$_GET['page']-2,"btn-outline-light",$_GET['page']-2);
    }
    if(($_GET['page']+1)-1>1){
        echo $comments->pageButtons($prev_link,$_GET['page']-1,"btn-outline-light",$_GET['page']-1);
    }
}
?>
<select class="form-control m-0 p-0 d-inline bg-transparent text-info border-info all-pages">
    <?php
    for ($i=1; $i < $all_pages+1; $i++) { 
        ?>
        <option class="text-dark" <?php
        if($_GET['page']==$i){
            echo "selected";
        }
        ?> value="<?php
        echo isset($_GET['songid'])?'/comments/song/':'/comments/singer/';
        echo isset($_GET['songid'])?$_GET['songid']:$_GET['singerid'];
        echo'/'.$i;
        ?>">
            <?php
                echo $i;
            ?>
        </option>
        <?php
    }
    ?>
</select>
<?php
if($all_pages>$_GET['page']){
    if($page_diff>=1){
        echo $comments->pageButtons($next_link,$_GET['page']+1,"btn-outline-light",$_GET['page']+1);
    }
    if($page_diff>=2){
        echo $comments->pageButtons($next_next_link,$_GET['page']+2,"btn-outline-light",$_GET['page']+2);
    }
    if($page_diff>=1){
        echo $comments->pageButtons($next_link,"Next","btn-outline-light","<i class='fas fa-angle-right'></i>");
    }
    if($page_diff>=3){
        echo $comments->pageButtons($last_link,"Last","btn-outline-light","<i class='fas fa-angle-double-right'></i>");
    }
}
?>
</div>

There's a short Jquery code for the select to work, which is the following:

 $(".all-pages").on("change",function(){
    $(this).find("option").each(function(){
        if($(this).is(":selected")){
            location.href=$(this).attr("value");
        }
    });
});

And a little bit of CSS

@media (max-width: 776px){
  .page-buttons-container .page-buttons{
    width:20px !important;
    border-radius: 100% !important;
    padding:0 !important;
  }
.all-pages{
  border-radius: 100%;
  width: 25px;
  height:25px;
  border-width: 2px;
  }
}
.all-pages {
  -webkit-appearance: none;
  -moz-appearance: none;
  text-indent: 1px;
  text-overflow: '';
  text-indent: 8px;
  width: 30px;
  height:30px;
  border-width: 2px;
}

I'd like to ask your opinion/review on this code. Is it good enough, or there are still ways to improve it, make it simpler, more understandable/readable, or anything else?

If you have any questions, or you don't understand something in the code, feel free to ask it.

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

First of all, it is very good you decided to ask.
Also, it is also very good you are using count(*) to get the count.

For the rest follow the review.

  1. There must be a single table with comments where the comment type is distinguished by means a dedicated field. Which makes your code would be

    function countComments($ID, $type){
        $st = $this->conn->prepare("SELECT COUNT(*) FROM comments WHERE ID=? and type=?");
        $st->execute([$ID, $type]);
        return $st->fetchColumn();
    }
    
  2. The code to get $total variable a good example of a cargo cult code.

    • why define $total if it gets assigned the line below?
    • why loop over a result that contains only one row?
    • I don't really get why do you need $all variable
    • Finally, PDO supports different fetch types, there is no point in using fetchAll() in countComments(). So the code should be just

      $total = $comments->countComments($ID, $type);
      
  3. I don't really get what does your counting the number of pages do, but all examples I've seen are doing it in a single move,

    $all_pages = ceil($total/$per_page);
    
  4. The code in Defining the links is essentially duplicated. Why not to define only different parts and then have a single block of code

    $type = isset($_GET['songid']) ? 'song' : 'singer';
    $id   = $_GET[$type.'id'];
    $base_link = "/comments/$type/$id/";
    
    $next_link      = $base_link.($_GET['page']+1);
    $prev_link      = $base_link.($_GET['page']-1);
    $next_next_link = $base_link.($_GET['page']+2);
    $prev_prev_link = $base_link.($_GET['page']-2);
    $last_link      = $base_link.$page;
    $first_link     = $base_link."1";
    

That's for starter, hope someone will cover the rest

\$\endgroup\$
1
  • \$\begingroup\$ Thank you for ypur answer, the code you provided definitely are better and simpler, especially the 4th one with the links, I really like it. I've realized I overcomplicated things, like counting the pages. About merging the two tables: it's been in plan for a while, and I wanted to solve it the same way you did. I've already did it with other tables(like comment reports), but I really just forgot about this one. \$\endgroup\$
    – K. P.
    Nov 9, 2019 at 18:21

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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