Return to Revisions

1. Is the code DRY enough?

It could be less repetitive. The following lines appear in both _initPagination() and byauthor()

if ($this->Static_model->get_static_data()['has_pager']) {
  $config['display_pages'] = FALSE;
  $config['first_link'] = FALSE;
  $config['last_link'] = FALSE;
  $config['prev_tag_open'] = '<li class="prev">';
  $config['prev_tag_close'] = '</li>';
  $config['next_tag_open'] = '<li class="next">';
  $config['next_tag_close'] = '</li>';
}
if (!isset($_GET[$config['query_string_segment']]) || $_GET[$config['query_string_segment']] < 1) {
  $_GET[$config['query_string_segment']] = 1;
}

Those could be abstracted into a separate method that can be called in both places.

Similarly the following lines appear in four places - in methods index, byauthor(), search(), post():

$data                 = $this->Static_model->get_static_data();
$data['base_url']     = base_url("/");
$data['pages']        = $this->Pages_model->get_pages();
$data['categories']   = $this->Categories_model->get_categories();

2. Are there any security risks?

You stated last year that you attempted to follow advice about CSRF protection and XSS filtering but the AJAX forms won't work. Were you able to resolve that? perhaps It didn't work because the AJAX forms weren't submitting the CSRF tokens. The token could be included with the AJAX data, or else the URIs could be excluded from CSRF protection (see the end of the CSRF protection in the documentation).

3. Improvement opportunities?

As I mentioned two years ago in my review of your Posts controller the Posts controller (still) has a constructor that merely calls the parent controller. Unless there is a good reason (e.g. provide/override specific parameters) there is not really a point in defining the overriden constructor and it can be eliminated.

Perhaps the constructor is overridden because the documentation mentions them:

Constructors are useful if you need to set some default values, or run a default process when your class is instantiated. Constructors can’t return a value, but they can do some default work.

However in the Posts constructor no default values are set and no default process is run when the class is instantiated, so the override can be removed.

In the _initPagination() method the single-use variables - i.e. $limit and $offset - can be eliminated:

return [
  'limit' => $config['per_page'],
  'offset' => ($this->input->get($config['query_string_segment']) - 1) * $limit
];

Note the shorter array syntax is used, presuming PHP 5.4+ is used - hopefully it is 7.3 or newer for the sake of support.

In the Javascript code:

pageBaseUrl = pageUrl.split('?')[0],
searchStr = pageUrl.split('?')[1];

The string is being split twice, which isn’t as bad as splitting it many times but it could be done with a single split- e.g.

const [pageBaseUrl, searchStr] = pageUrl.split('?');

Or instead of parsing it manually, the URL interface could be used - including modifying the query string with the URLSearchParams get method on the URL.searchParams property and the href property (or the ``toString()`](https://developer.mozilla.org/en-US/docs/Web/API/URL/toString) method).

As I mentioned in this review the AJAX method (i.e. type parameter) can be eliminated by using a shorthand method - i.e. $.get().

And the incrementing of the page number can be simplified from:

currentPage = currentPage + 1;

To

currentPage++;

In fact it can be changed to a prefix increment in the conditional:

 if (++currentPage > maxPage) {