0
\$\begingroup\$

I have created a function that helps me generate pagination really easily but I am concerned about its performance mainly.

The function accepts 2 parameters:

  1. A list of items (has meaning in the framework I'm using) which contains information about the state of pagination itself and state of the storage layer - more precisely:

    • page the current page
    • perpage how many items are being shown per page
    • total the total amount of items in storage.
  2. Parameter name - allows to customize the query string parameter key, which is by default named page - http://example.com/news?page=1.


public function paginationControls(BaseList $list, $parameter = 'page') {

    if (!isset($list->pagination)) {
        return 'Error: List has not been supplied with pagination details.';
    }

    // if total results are less than the results displayed on each page
    // then do not render controls
    if ($list->pagination->perpage >= $list->pagination->total) {
        return null;
    }

    // Keep track of current query string parameters
    $query = trim(preg_replace('/' . $parameter . '\=\d+\&?/', '', $_SERVER['QUERY_STRING']), '&');
    if (strlen($query)) {
        $query .= '&';
    }
    $query .= $parameter . '=';

    $makeanchor = function($page, $current = false) use ($query) {
        if ($current) {
            $url = '#';
            $class = 'pagination anchor current';
        } else {
            $url = '?' . $query . $page;
            $class = 'pagination anchor';
        }

        return $this->a($url, $page + 1, null, ['class' => $class]);
    };

    $page = $list->pagination->page;
    $pages = ceil($list->pagination->total / $list->pagination->perpage);

    // If page is greater than 3 then display link to first page
    $output = $page > 3 ? ($makeanchor(0) . ' ... ') : '';

    // Add controls for 3 pages before current page
    $tmp = '';
    for ($i = 1; $i <= 3 && $page - $i >= 0; $i++) {
        $tmp = $makeanchor($page - $i) . $tmp;
    }

    // Add current page indicator
    $output .= $tmp . $makeanchor($page, true);

    // Add controls for 3 pages after current page
    for ($i = 1; $i <= 3 && $page + $i + 1 <= $pages; $i++) {
        $output .= $makeanchor($page + $i);
    }

    // If there are more than 3 pages after the current page
    // Then add control for last page
    if ($page + $i < $pages) {
        $output .= ' ... ' . $makeanchor($pages - 1);
    }

    return $output;
}

And here are the respective helper functions

public function a($url, $text = null, $target = null, array $attributes = array()) {

    $attributes['href'] = $url;

    if ($target !== null) {
        $attributes['target'] = $target;
    }

    if ($text === null) {
        $text = $attributes['href'];
    }

    return '<a' . $this->parseAttributes($attributes) . '>' . html($text) . '</a>';
}

private function parseAttributes($attributes) {
    $string = '';

    foreach ($attributes as $attribute => $value) {
        $string .= ' ' . html($attribute) . '="' . html($value) . '"';
    }

    return $string;
}

In a class far, far away

function html($string, $flag = ENT_QUOTES, $encoding = 'UTF-8') {
    return htmlspecialchars($string, $flag, $encoding);
}

I'm mainly interested in optimizing this function in terms of performance, it would be very much appreciated if someone more experienced than me can have a say on this.

\$\endgroup\$
  • \$\begingroup\$ I like your code, eww, you sexy lady! \$\endgroup\$ – coderodde Jan 27 '16 at 15:33
1
\$\begingroup\$

Your code looks clean. The suggested changes below mainly focuses on reducing it a bit, so IMO improving its readability.
But I'm not quite sure it'll also improve performance as you said you're interested to: I only hope some light gain could come from having suppressed your $makeanchor() function.

Changes I propose affect only your main paginationControl() function:

public function paginationControls(BaseList $list, $parameter = 'page') {

    if (!isset($list->pagination)) {
        return 'Error: List has not been supplied with pagination details.';
    }

    // if total results are less than the results displayed on each page
    // then do not render controls
    if ($list->pagination->perpage >= $list->pagination->total) {
        return null;
    }

    // Keep track of current query string parameters
    $query = preg_replace('/' . $parameter . '\=\d+\&?/', '', $_SERVER['QUERY_STRING']);
    if ($query) {
        $query .= '&';
    }
    $query .= $parameter . '=';

    $page = $list->pagination->page;
    $pages = ceil($list->pagination->total / $list->pagination->perpage);

    // Prepare link to first page, if needed
    if ($page > 3) {
      $pageSet['1...'] = FALSE;
    }
    // Prepare link to 3 pages before + current + 3 pages after
    for ($i = $page - 3; $i <= $page + 3; $i++) {
      if ($i AND $i <= $pages) {
        $pageSet[$i] = ($i == $page);
      }
    }
    // Prepare link to last page, if needed
    if ($i < $pages) {
      $pageSet['...' . $pages] = FALSE;
    }

    // Build output
    $output = NULL;
    $classes = 'pagination anchor';
    foreach ($pageSet as $pageNo => $current) {
      $output .= $this->a(
        ($current ? '#' : '?' . $query . $pageNo),
        $pageNo,
        NULL,
        ($classes . ($current ? 'current' : NULL))
      );
    }

    return $output;
}

Apart from the way to build pagination, you'll notice that:

  • I dropped trim(..., '&') from your $query = ..., since "&" has already suppressed by preg_replace(); otherwise it sounds like you suspect the whole query to abusively include an ending "&"
  • at the just next line, I dropped strlen() from if ($query), which is enough (and may be slightly faster, but it is only cosmetic)
| improve this answer | |
\$\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.