HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Generating pagination links

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
linkspaginationgenerating

Problem

I recently had an interview for a programming job and after the interview I was then asked to create a function with the following brief:

The Task:

Relating to pagination links: given a page number, a total number of pages, and an amount of 'context' pages, generate the appropriate pagination links. For example, with 9 pages in total and a context of 1 page, you would get the following results for pages 1 to 9:

For page 1: 1 2 ... 8 9

For page 2: 1 2 3 ... 8 9

For page 3: 1 2 3 4 ... 8 9

For page 4: 1 2 3 4 5 ... 8 9

For page 5: 1 2 ... 4 5 6 ... 8 9

For page 6: 1 2 ... 5 6 7 8 9

For page 7: 1 2 ... 6 7 8 9

For page 8: 1 2 ... 7 8 9

For page 9: 1 2 ... 8 9

I would like you to write a PHP function with the following signature to solve this task:

function outputLinks($page, $numberOfPages, $context)


The code below is what I submitted and I've now been asked to complete another task which is good news in my eyes, so I can only assume that my code for this task was at least good enough. But I was just wondering how good my submission really was! Is there an easier (or simpler) way to achieve the same results? Could I shorten my code at all?

```
$numberOfPages) { $page = $numberOfPages; }

// Set array variables

$leftArray = $midArray = $rightArray = array();

// Left Array

for ($x = 1; $x 0; $x++) {
if (!in_array($x, $leftArray)) {
array_push($rightArray, $x);
}
}

// Left/Right Array First/Last values to variables

$firstInLeftArray = current($leftArray);
$lastInLeftArray = end($leftArray);
$firstInRightArray = current($rightArray);
$lastInRightArray = end($rightArray);

// Middle Array

if ($page == $firstInLeftArray || $page == $lastInRightArray) {

if (($firstInRightArray - $lastI

Solution

Yes, I would have written this completely differently (so I decided to do just that). I'll follow my solution with a hopefully scientific analysis of both solutions. Finally I'll give some personal recommendations.

Note: The reason for the in-depth analysis is that I was already interested in pagination and thinking about an implementation for a framework that I am writing.

Model

/** Model the data for pagination.
 */
class ModelPaginator
{
    /** Get the links for pagination.
     *  @param page \int The page number for the current page.
     *  @param numberOfPages \int The total number of paginated pages.
     *  @param context \int The number of context pages surrounding the current,
     *  first and last pages.
     *  @return \array An array with the ranges of pages for pagination.
     */
    public function getLinks($page, $numberOfPages, $context)
    {
        // Sanitize the inputs (I am trying to follow your example here, I would
        // throw exceptions if the values weren't as I expected).
        $numberOfPages = max($numberOfPages, 1);
        $context       = min(max($context, 0), $numberOfPages - 1);
        $page          = min(max($page, 1), $numberOfPages);

        return array_unique(
            array_merge(range(1, 1 + $context),
                        range(max($page - $context, 1),
                              min($page + $context, $numberOfPages)),
                        range($numberOfPages - $context, $numberOfPages)));
    }
}


View

/** A simple view for paginator links.
 */
class ViewPaginatorLinks
{
    protected $separator;

    /** Construct the paginator.
     *  @param separator \string Separator between gaps.
     */
    public function __construct($separator=' ... ')
    {
        $this->separator = $separator;
    }

    /** Write the pagination links.
     *  @param links \array The links array.
     *  @param currentPage \int The current page.
     */
    public function write($links, $currentPage)
    {
        $currentPage = min(max($currentPage, 1), end($links));

        echo 'Page ' . $currentPage . ' of ' . end($links) . ': ';
        $previousPage = 0;

        foreach ($links as $page) {
            if ($page !== $previousPage + 1) {
                $this->writeSeparator();
            }

            $this->writeLink($page);
            $previousPage = $page;
        }
    }

    /*******************/
    /* Private Methods */
    /*******************/

    /** Write the link to the paginated page.
     *  @param page \array The page that we are writing the link for.
     */
    private function writeLink($page)
    {
        echo '' . $page . '' .
            "\n";
    }

    /// Write the separator that bridges gaps in the pagination.
    private function writeSeparator()
    {
        echo '' . $this->separator . '' . "\n";
    }
}


Usage

$numberOfPages = 29;
$currentPage = 13;
$context = 1;

$Model = new ModelPaginator;
$View = new ViewPaginatorLinks;

$View->write($Model->getLinks($currentPage, $numberOfPages, $context),
             $currentPage);


Analysis

I'll be going through some metrics thanks to PHPLOC. I added the maximum line length to these metrics.

Your Solution

Lines of Code (LOC):                                117
  Cyclomatic Complexity / Lines of Code:           0.37
Comment Lines of Code (CLOC):                        34
Non-Comment Lines of Code (NCLOC):                   83

Maximum Line Length                                 120


My Solution

Lines of Code (LOC):                                 97
  Cyclomatic Complexity / Lines of Code:           0.03
Comment Lines of Code (CLOC):                        34
Non-Comment Lines of Code (NCLOC):                   63

Maximum Line Length                                  80


Lines of Code

My solution has 20 fewer lines. This could well be due to removing quite a few blank lines. However, measured in characters it is 2905 characters long against your 4089. So there is definitely a large difference in typing required.

Cyclomatic Complexity

This is a measure of how complex the code is (see wikipedia). Highly complex code is normally harder to maintain and contains more bugs. My code is a factor of 10 less complex. This is due to the flatness of my code. See Jeff Atwood's Flattening Arrow Code blog.

Comments

The number has remained the same according to PHPLOC, but it seems to be counting blank lines. Manually I can only count 14 in your code versus 27 in mine, even though the number of lines of code were reduced in my solution. Proportionally the commenting in my solution is much higher. At the highest level of abstraction (class, function definitions) I have comments where you have virtually none. My comments cover input parameters to functions and can be used in automatic documentation tools like Doxygen or PHPDocumentor.

Line Length

Firstly, I am unsure whether the formatting of your code here is exactly as it was.

Code Snippets

/** Model the data for pagination.
 */
class ModelPaginator
{
    /** Get the links for pagination.
     *  @param page \int The page number for the current page.
     *  @param numberOfPages \int The total number of paginated pages.
     *  @param context \int The number of context pages surrounding the current,
     *  first and last pages.
     *  @return \array An array with the ranges of pages for pagination.
     */
    public function getLinks($page, $numberOfPages, $context)
    {
        // Sanitize the inputs (I am trying to follow your example here, I would
        // throw exceptions if the values weren't as I expected).
        $numberOfPages = max($numberOfPages, 1);
        $context       = min(max($context, 0), $numberOfPages - 1);
        $page          = min(max($page, 1), $numberOfPages);

        return array_unique(
            array_merge(range(1, 1 + $context),
                        range(max($page - $context, 1),
                              min($page + $context, $numberOfPages)),
                        range($numberOfPages - $context, $numberOfPages)));
    }
}
/** A simple view for paginator links.
 */
class ViewPaginatorLinks
{
    protected $separator;

    /** Construct the paginator.
     *  @param separator \string Separator between gaps.
     */
    public function __construct($separator=' ... ')
    {
        $this->separator = $separator;
    }

    /** Write the pagination links.
     *  @param links \array The links array.
     *  @param currentPage \int The current page.
     */
    public function write($links, $currentPage)
    {
        $currentPage = min(max($currentPage, 1), end($links));

        echo 'Page ' . $currentPage . ' of ' . end($links) . ': ';
        $previousPage = 0;

        foreach ($links as $page) {
            if ($page !== $previousPage + 1) {
                $this->writeSeparator();
            }

            $this->writeLink($page);
            $previousPage = $page;
        }
    }

    /*******************/
    /* Private Methods */
    /*******************/

    /** Write the link to the paginated page.
     *  @param page \array The page that we are writing the link for.
     */
    private function writeLink($page)
    {
        echo '<a href="?p=' . $page . '" target="_self">' . $page . '</a>' .
            "\n";
    }

    /// Write the separator that bridges gaps in the pagination.
    private function writeSeparator()
    {
        echo '<span>' . $this->separator . '</span>' . "\n";
    }
}
$numberOfPages = 29;
$currentPage = 13;
$context = 1;

$Model = new ModelPaginator;
$View = new ViewPaginatorLinks;

$View->write($Model->getLinks($currentPage, $numberOfPages, $context),
             $currentPage);
Lines of Code (LOC):                                117
  Cyclomatic Complexity / Lines of Code:           0.37
Comment Lines of Code (CLOC):                        34
Non-Comment Lines of Code (NCLOC):                   83

Maximum Line Length                                 120
Lines of Code (LOC):                                 97
  Cyclomatic Complexity / Lines of Code:           0.03
Comment Lines of Code (CLOC):                        34
Non-Comment Lines of Code (NCLOC):                   63

Maximum Line Length                                  80

Context

StackExchange Code Review Q#10271, answer score: 6

Revisions (0)

No revisions yet.