Skip to main content
Code Review

Return to Answer

added 111 characters in body
Source Link
Sᴀᴍ Onᴇᴌᴀ ♦
Sᴀᴍ Onᴇᴌᴀ ♦
  • 29.6k
  • 16
  • 46
  • 203

The indentation is inconsistent - especially in the JavaScript code - which makes it difficult to read.

I see that you have a previous post with identical code, thus this post is a follow-up to that one. That post has an accepted answer, though not all suggestions were taken into account. This is fine but you might want to consider those suggestions.

unused variable page in event listener function

window.addEventListener('popstate', e => {
        var page = $(this).attr("id");
        load_data(e.state.page);
        console.log('popstate error!');
});

The variable page isn't used within the callback function. It can be eliminated.

I see that you have a previous post with identical code, thus this post is a follow-up to that one. That post has an accepted answer, though not all suggestions were taken into account. This is fine but you might want to consider those suggestions.

The indentation is inconsistent - especially in the JavaScript code - which makes it difficult to read.

I see that you have a previous post with identical code, thus this post is a follow-up to that one. That post has an accepted answer, though not all suggestions were taken into account. This is fine but you might want to consider those suggestions.

unused variable page in event listener function

window.addEventListener('popstate', e => {
        var page = $(this).attr("id");
        load_data(e.state.page);
        console.log('popstate error!');
});

The variable page isn't used within the callback function. It can be eliminated.

deleted 388 characters in body
Source Link
Sᴀᴍ Onᴇᴌᴀ ♦
Sᴀᴍ Onᴇᴌᴀ ♦
  • 29.6k
  • 16
  • 46
  • 203

Sanitizing inputs

I altered the id attribute of one of the paging links to -3 using my browser console, then clicked the link. The page showed "Database query failed.". This is fine but it may be better to show a message about invalid inputs. It may be best to ensure that $page has a positive integer, as opposed to a negative number - or worse - a string or some other type

Constants

Sanitizing inputs

I altered the id attribute of one of the paging links to -3 using my browser console, then clicked the link. The page showed "Database query failed.". This is fine but it may be better to show a message about invalid inputs. It may be best to ensure that $page has a positive integer, as opposed to a negative number - or worse - a string or some other type

Constants

Constants

Source Link
Sᴀᴍ Onᴇᴌᴀ ♦
Sᴀᴍ Onᴇᴌᴀ ♦
  • 29.6k
  • 16
  • 46
  • 203

Overall Assessment

It appears to work, although when a user on the demo site (presuming the code is the same) goes to page 10, only the link with text BACK is present. Should other page numbers exist? I understand why no other numbered links appear after having looked at the PHP logic but my experience with other paging systems makes me question this UI experience.

The practice of sending HTML in an AJAX response and using that to directly set the content of a DOM element is an antiquated practice, and could be an XSS avenue. While the practice of sending data from the API and having the front end code construct the HTML dynamically may have been a new construct eight years ago it is much more common in today’s web.

I see that you have a previous post with identical code, thus this post is a follow-up to that one. That post has an accepted answer, though not all suggestions were taken into account. This is fine but you might want to consider those suggestions.

By the way you might not need jQuery...

Feedback/Suggestions

Javascript

DOM ready callback

If you are using jQuery 3.0 or newer then the format for .ready() can be simplified to "the recommended syntax"3:

$(function() { ... })

Instead of:

$(document).ready(function(){

Parsing page parameter

Instead of using window.location.href the search property could be used. On that MDN page it states:

Modern browsers provide URLSearchParams and URL.searchParams to make it easy to parse out the parameters from the querystring.

Your code could utilize the URLSearchParams interface - something along the lines of:

const params = new URLSearchParams(document.location.search.substring(1));
const name = params.get("page") || 1;

jQuery AJAX functions

There is a convenience method .post() that could be used to simplify the code to run the AJAX request - for example, I haven't tested this but it could be as simple as this:

const container = $(#load_data);
container.html('<div id="status" style="" ></div>');
$.post("pagination2.php", { page }, container.html);

PHP

Sanitizing inputs

I altered the id attribute of one of the paging links to -3 using my browser console, then clicked the link. The page showed "Database query failed.". This is fine but it may be better to show a message about invalid inputs. It may be best to ensure that $page has a positive integer, as opposed to a negative number - or worse - a string or some other type

Constants

Things that shouldn't be changed (e.g. number of rows per page) can be declared as a constant - e.g. with define() or const.

const ROWS_PER_PAGE = 10;

One thing that was already mentioned in the answer by @mickmackusa to your previous question is that:

The LIMIT clause string can be written without concatenation:
$sql .= "LIMIT $p, $rowperpage";

I see that code is still unchanged. Perhaps it be helpful to know the reason: Variables in double-quoted (and heredoc syntaxes) are expanded when they occur in double quoted strings.

Closing and re-opening PHP tags

I see this code at the top of pagination2.php:

<?php require_once('../private/initialize.php'); ?>

<?php

$page = '';

There is little need to close and re-open the PHP tags.

<?php 
require_once('../private/initialize.php'); 
$page = '';