2
\$\begingroup\$

A little birdie suggested I bring this question here, so here it goes.

Well I have a working script (see below) but it seems quite clunky and redundant; in my defense I wrote this code many moons ago, but that's not the point. I was curious if anyone has an idea on a more efficient way of writing this code, with less loops and conditionals and, well, noise in the code.

Code in question:

private function pageLinks($num, $page = 1, $search = false, $ne = false) {
    $query = ($search) ? '&query='.$search : null;
    $by = (is_numeric($ne)) ? '&by='.$ne : null;
    $links = 'Page(s):<a href="search.php?page=1' . $query . $by . '" class="tableLink">1</a>';
    $count = 1;
    $npp = $this->numPerPage;
    $buttons = 9;
    $half = 4;
    for($i = 1; $i <= $num; $i++) {
        if(($i%$npp) === 0) {
            $count++;
        }
    }
    if($count < $buttons) {
        for($i = 2; $i <= $count; $i++) {
            $links .= '<a href="search.php?page=' . $i . $query . $by . '" class="tableLink">' . $i . '</a>';
        }
    } elseif($page <= ($half + 2)) {
        for($i = 2; $i <= $buttons; $i++) {
            $links .= '<a href="search.php?page=' . $i . $query . $by . '" class="tableLink">' . $i . '</a>';
        }
        $links .= '...<a href="search.php?page=' . $count . $query . $by . '" class="tableLink">' . $count . '</a>';
    } elseif($page <= ($count - ($half + 2))) {
        $links .= '...';
        for($i = $half; $i > 0; $i--) {
            $links .= '<a href="search.php?page=' . ($page - $i) . $query . $by . '" class="tableLink">' . ($page - $i) . '</a>';
        }
        $links .= '<a href="search.php?page=' . ($page - $i) . $query . $by . '" class="tableLink">' . ($page - $i) . '</a>';
        for($i = 1; $i <= $half; $i++) {
            $links .= '<a href="search.php?page=' . ($page + $i) . $query . $by . '" class="tableLink">' . ($page + $i) . '</a>';
        }
        $links .= '...<a href="search.php?page=' . $count . $query . $by . '" class="tableLink">' . $count . '</a>';
    } else {
        $links .= '...';
        for($i = $buttons - 1; $i >= 0; $i--) {
            $links .= '<a href="search.php?page=' . ($count - $i) . $query . $by . '" class="tableLink">' . ($count - $i) . '</a>';
        }
    }
    return($links);
}

The method is called like so:

$links = $this->pageLinks($count, $page, $url, $ne);

And the variables are as such:

$count = total number of clients in database (int)
$page = current page to build from (int)
$url = the name or email for the search (String)
$ne = is for the search string either by name (1) or email (2) (int)

And the output is something like (as links):

Page(s):1 2 3 4 5 6 7 8 9...33

Or if you're in the middle (page 20):

Page(s):1...16 17 18 19 20 21 22 23 24...33

Now this isn't always called through a search function, hence the default values for $url and $ne, but that's not very important. My question is there a cleaner way to handle building of these links? Or am I stuck with this cluster of loops?

\$\endgroup\$
1
\$\begingroup\$

This:

$count = 1;
for($i = 1; $i <= $num; $i++) {
    if(($i%$npp) === 0) {
        $count++;
    }
}

is equivalent to

$count = floor($num / $npp) + 1;

As for the paging, I'd do it like this:

$from = $page - $half;
if ($from <= 2) $from = 2;
$to = $page + $half;
if ($to >= $count - 1) $to = $count - 1;

$extra = $query . $by;
$links = $this->pageLink(1, $extra);
if ($from > 2) $links .= "...";
for ($i = $from; $i <= $to; $i++)
    $links .= $this->pageLink($i, $extra);
if ($i < $count) $links .= "..."; // I use $i instead of $to because $i == $to + 1, so I save one addition
$links = $this->pageLink($count, $extra);

Here, you also need:

private function pageLink($num, $extra) {
    return '<a href="search.php?page=' . $num . $extra . '" class="tableLink">' . $num . '</a>';
}

This was written by heart, so be weary of the possible bugs.

| improve this answer | |
\$\endgroup\$
  • 1
    \$\begingroup\$ Genius! Works like a charm! Only bug was a missing $ in the pageLink method, but that's the least of my worries. Wow, it's.. beautiful. Thank you, very informative and exactly what I was looking for. \$\endgroup\$ – faino Aug 17 '13 at 23:14
  • \$\begingroup\$ I've fixed it. Thank you for that, and I'm glad I've been able to help. \$\endgroup\$ – Vedran Šego Aug 17 '13 at 23:43

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.