1
\$\begingroup\$

I've re-written my old pagination class into something a little cleaner, and also added PDO support (The prev version was mysqli only). I'd like to clean it up even more if it's possible, does anyone have any pointers?

Here's the class:

<?

class paginate
{


    /**
     * Array of options for the class
     *
     * @access public
     * @var    array
     */
    public $options = array(
        'results_per_page'              => 10,
        'url'                           => '',
        'url_page_number_var'           => '*VAR*',
        'text_prev'                     => '&laquo; Prev',
        'text_next'                     => 'Next &raquo;',
        'text_first'                    => '&laquo; First',
        'text_last'                     => 'Last &raquo;',
        'text_ellipses'                 => '...',
        'class_ellipses'                => 'ellipses',
        'class_dead_links'              => 'dead-link',
        'class_live_links'              => 'live-link',
        'class_current_page'            => 'current-link',
        'class_ul'                      => 'pagination',
        'show_links_first_last'         => true,
        'show_links_prev_next'          => true,
        'show_links_first_last_if_dead' => true,
        'show_links_prev_next_if_dead'  => true,
        'max_links_between_ellipses'    => 7,
        'max_links_outside_ellipses'    => 2,
        'db_conn_type'                  => 'mysqli',  /* Can be either: 'mysqli' or 'pdo' */
        'db_handle'                     => null
    );


    /**
     * An array of any errors
     *
     * @access public
     * @var    array
     */
    public $debug_log;


    /**
     * The current page
     *
     * @access public
     * @var    int
     */
    public $current_page;


    /**
     * The query to run on the database
     *
     * @access public
     * @var    string
     */
    public $query;


    /**
     * The resultset of the query
     *
     * @access public
     * @var    resultset
     */
    public $resultset;


    /**
     * The total results of the query
     *
     * @access public
     * @var    int
     */
    public $total_results;


    /**
     * The total pages returned
     *
     * @access public
     * @var    int
     */
    public $total_pages;


    /**
     * The total total number of links to render before showing the ellipses
     *
     * @access public
     * @var    int
     */
    public $number_of_links_before_showing_ellipses;


    /**
     * The pagination links (Presented as an UL)
     *
     * @access public
     * @var    string
     */
    public $links_html;


   /**
    * __construct(int $surrent_page, string $query, array $options)
    *
    * Class constructor
    *
    * @access  public
    * @param   int     $current_page  The number of the current page (Starts at 1)
    * @param   string  $query         The query to run on the database
    * @param   array   $options       An array of options
    * @return  void
    */
    public function __construct($current_page = 1, $query = '', $options = null)
    {
        /*
         * Set the current page
         */
        $this->current_page = $current_page;

        /*
         * Set the query to run
         */
        $this->query = $query;

        /*
         * Populate the options array
         */
        if(!empty($options))
        {
            foreach($options as $key => $value)
            {
                if(array_key_exists($key, $this->options))
                {
                    $this->options[$key] = $value;
                }
                else
                {
                    $this->debug_log[] = 'Attempted to add setting \''.$key.'\' with the value \''.$value.'\' - option does not exist';
                }
            }
        }

        /*
         * Check to make sure 'max_links_between_ellipses' is an odd number
         */
        if(!($this->options['max_links_between_ellipses'] & 1))
        {
            $this->debug_log[] = 'Setting \'max_links_between_ellipses\' has been set with the value \''.$this->options['max_links_between_ellipses'].'\' - This number must be an odd number';
            echo 'Setting \'max_links_between_ellipses\' has been set with the value \''.$this->options['max_links_between_ellipses'].'\' - This number must be an odd number';
        }


        $this->prepare_query();

        $this->run_query();

        $this->calculate_number_of_pages();

        $this->calculate_max_pages_before_ellipses();

        $this->build_links();
    }


    /**
    * prepare_query(void)
    *
    * Prepares the query to be run with the found rows and start/end limits
    *
    * @access  public
    * @return  void
    */
    public function prepare_query()
    {
        /*
         * Add SQL_CALC_FOUND_ROWS for finding out total amount of results later on
         */
        $this->query = substr_replace($this->query, 'SELECT SQL_CALC_FOUND_ROWS', 0, 6);

        /*
         * Add our start/end limit
         */
        if($this->current_page == 1)
        {
            $this->query .= ' LIMIT 0, '.$this->options['results_per_page'];
        }
        else
        {
            $this->query .= ' LIMIT '.(($this->current_page - 1) * $this->options['results_per_page']).', '.$this->options['results_per_page'];
        }
    }


    /**
    * run_query(void)
    *
    * Run's the query against the database
    *
    * @access  public
    * @return  void
    */
    public function run_query()
    {
        if($this->options['db_conn_type'] == 'mysqli')
        {
            /*
             * Execute using MySQLi
             */
            $this->resultset = $this->options['db_handle']->query($this->query);

            /*
             * Get the total results with FOUND_ROWS()
             */
            $count_rows = $this->options['db_handle']->query('SELECT FOUND_ROWS();');
            $found_rows = $count_rows->fetch_assoc();
            $this->total_results = $found_rows['FOUND_ROWS()'];
        }
        elseif($this->options['db_conn_type'] == 'pdo')
        {
            /*
             * Execute using PDO
             */
            $pdos = $this->options['db_handle']->prepare($this->query);
            $pdos->execute();
            $this->resultset = $pdos;

            /*
             * Get the total results with FOUND_ROWS()
             */
            $pdos_fr = $this->options['db_handle']->prepare("SELECT FOUND_ROWS();");
            $pdos_fr->execute();
            $pdos_fr_result = $pdos_fr->fetch(PDO::FETCH_ASSOC);
            $this->total_results = $pdos_fr_result['FOUND_ROWS()'];
        }
        else
        {
            /*
             * An unknown DB connection type has been set
             */
            $this->debug_log[] = 'You have selected a \'db_conn_type\' of \''.$this->options['db_conn_type'].'\' - this method is not supported';
        }
    }


    /**
    * calculate_number_of_pages(void)
    *
    * Calculates how many pages there will be
    *
    * @access  public
    * @return  void
    */
    public function calculate_number_of_pages()
    {
        $this->total_pages = ceil($this->total_results / $this->options['results_per_page']);
    }


    /**
    * calculate_max_pages_before_ellipses(void)
    *
    * Calculates the number of links to show before showing an ellipses
    *
    * @access  public
    * @return  void
    */
    public function calculate_max_pages_before_ellipses()
    {
        $this->number_of_links_before_showing_ellipses = $this->options['max_links_between_ellipses'] + ($this->options['max_links_outside_ellipses'] * 2);
    }


    /**
    * build_link_url(int $page_number)
    *
    * Builds the URL to insert in links
    *
    * @access  public
    * @param   int     $page_number  The page number to insert into the link
    * @return  string                The built URL
    */
    public function build_link_url($page_number)
    {
        return str_replace($this->options['url_page_number_var'], $page_number, $this->options['url']);
    }


    /**
    * get_current_or_normal_class(int $page_number)
    *
    * Returns the live link class, or link link and current page class
    *
    * @access  public
    * @param   int     $page_number  The page number to insert into the link
    * @return  string                The class to use
    */
    public function get_current_or_normal_class($page_number)
    {
        if($page_number == $this->current_page)
        {
            return $this->options['class_live_links'].' '.$this->options['class_current_page'];
        }
        else
        {
            return $this->options['class_live_links'];
        }
    }


    /**
    * build_links(void)
    *
    * Build the HTML links
    *
    * @access  public
    * @return  void
    */
    public function build_links()
    {
        /*
         * Start the UL
         */
        $this->links_html = '<ul class="'.$this->options['class_ul'].'">'.PHP_EOL;

        /*
         * The 'First' link
         */
        if($this->options['show_links_first_last'] == true)
        {
            if($this->current_page == 1 && $this->options['show_links_first_last_if_dead'] == true)
            {
                $this->links_html .= '<li><span class="'.$this->options['class_dead_links'].'">'.$this->options['text_first'].'</span></li>'.PHP_EOL;
            }
            elseif($this->current_page != 1)
            {
                $this->links_html .= '<li><a class="'.$this->options['class_live_links'].'" href="'.$this->build_link_url(1).'">'.$this->options['text_first'].'</a></li>'.PHP_EOL;
            }
        }

        /*
         * The 'Previous' link
         */
        if($this->options['show_links_prev_next'] == true)
        {
            if($this->current_page == 1 && $this->options['show_links_prev_next_if_dead'] == true)
            {
                $this->links_html .= '<li><span class="'.$this->options['class_dead_links'].'">'.$this->options['text_prev'].'</span></li>'.PHP_EOL;
            }
            elseif($this->current_page != 1)
            {
                $this->links_html .= '<li><a class="'.$this->options['class_live_links'].'" href="'.$this->build_link_url($this->current_page - 1).'">'.$this->options['text_prev'].'</a></li>'.PHP_EOL;
            }
        }

        /*
         * Build our main links
         */
        if($this->total_pages <= $this->number_of_links_before_showing_ellipses)
        {
            /*
             * If there's not enough links to have an ellipses in the set, just run through them all
             */
            $counter = 1;

            while($counter <= $this->total_pages)
            {
                $this->links_html .= '<li><a href="'.$this->build_link_url($counter).'" class="'.$this->get_current_or_normal_class($counter).'">'.$counter.'</a></li>'.PHP_EOL;

                $counter++;
            }
        }
        else
        {
            /*
             * We have enough links to show the ellipses, so run through other method
             */
            if($this->current_page <= ($this->options['max_links_between_ellipses'] + $this->options['max_links_outside_ellipses']))
            {
                /*
                 * Type 1 - skipping the first ellipses due to being low in the current page number
                 */
                $counter = 1;

                while($counter <= ($this->options['max_links_between_ellipses'] + $this->options['max_links_outside_ellipses']))
                {
                    $this->links_html .= '<li><a href="'.$this->build_link_url($counter).'" class="'.$this->get_current_or_normal_class($counter).'">'.$counter.'</a></li>'.PHP_EOL;

                    $counter++;
                }

                $this->links_html .= '<li><span class="'.$this->options['class_ellipses'].'">'.$this->options['text_ellipses'].'</span></li>'.PHP_EOL;

                $counter = ($this->total_pages - $this->options['max_links_outside_ellipses']) + 1;

                while($counter <= $this->total_pages)
                {
                    $this->links_html .= '<li><a href="'.$this->build_link_url($counter).'" class="'.$this->get_current_or_normal_class($counter).'">'.$counter.'</a></li>'.PHP_EOL;

                    $counter++;
                }
            }
            elseif($this->current_page > ($this->options['max_links_between_ellipses'] + $this->options['max_links_outside_ellipses']) && $this->current_page < ($this->total_pages - ($this->options['max_links_between_ellipses'] + $this->options['max_links_outside_ellipses']) + 1))
            {
                /*
                 * Type 2 - Current page is between both sets of ellipses
                 */
                $counter = 1;

                while($counter <= $this->options['max_links_outside_ellipses'])
                {
                    $this->links_html .= '<li><a href="'.$this->build_link_url($counter).'" class="'.$this->get_current_or_normal_class($counter).'">'.$counter.'</a></li>'.PHP_EOL;

                    $counter++;
                }

                /*
                 * Pop in an ellipses
                 */
                $this->links_html .= '<li><span class="'.$this->options['class_ellipses'].'">'.$this->options['text_ellipses'].'</span></li>'.PHP_EOL;

                $before_after = (($this->options['max_links_between_ellipses'] - 1) / 2);

                $counter = $this->current_page - $before_after;

                while($counter <= $this->current_page + $before_after)
                {
                    $this->links_html .= '<li><a href="'.$this->build_link_url($counter).'" class="'.$this->get_current_or_normal_class($counter).'">'.$counter.'</a></li>'.PHP_EOL;

                    $counter++;
                }

                /*
                 * Pop in an ellipses
                 */
                $this->links_html .= '<li><span class="'.$this->options['class_ellipses'].'">'.$this->options['text_ellipses'].'</span></li>'.PHP_EOL;

                $counter = ($this->total_pages - $this->options['max_links_outside_ellipses']) + 1;

                while($counter <= $this->total_pages)
                {
                    $this->links_html .= '<li><a href="'.$this->build_link_url($counter).'" class="'.$this->get_current_or_normal_class($counter).'">'.$counter.'</a></li>'.PHP_EOL;

                    $counter++;
                }
            }
            else
            {
                /*
                 * Type 1 - skipping the last ellipses due to being high in the current page number
                 */
                $counter = 1;

                while($counter <= $this->options['max_links_outside_ellipses'])
                {
                    $this->links_html .= '<li><a href="'.$this->build_link_url($counter).'" class="'.$this->get_current_or_normal_class($counter).'">'.$counter.'</a></li>'.PHP_EOL;

                    $counter++;
                }

                $this->links_html .= '<li><span class="'.$this->options['class_ellipses'].'">'.$this->options['text_ellipses'].'</span></li>'.PHP_EOL;

                $counter = ($this->total_pages - ($this->options['max_links_between_ellipses'] + $this->options['max_links_outside_ellipses'])) + 1;

                while($counter <= $this->total_pages)
                {
                    $this->links_html .= '<li><a href="'.$this->build_link_url($counter).'" class="'.$this->get_current_or_normal_class($counter).'">'.$counter.'</a></li>'.PHP_EOL;

                    $counter++;
                }
            }
        }

        /*
         * The 'Next' link
         */
        if($this->options['show_links_prev_next'] == true)
        {
            if($this->current_page == $this->total_pages && $this->options['show_links_prev_next_if_dead'] == true)
            {
                $this->links_html .= '<li><span class="'.$this->options['class_dead_links'].'">'.$this->options['text_next'].'</span></li>'.PHP_EOL;
            }
            elseif($this->current_page != $this->total_pages)
            {
                $this->links_html .= '<li><a class="'.$this->options['class_live_links'].'" href="'.$this->build_link_url($this->current_page + 1).'">'.$this->options['text_next'].'</a></li>'.PHP_EOL;
            }
        }

        /*
         * The 'Last' link
         */
        if($this->options['show_links_first_last'] == true)
        {
            if($this->current_page == $this->total_pages && $this->options['show_links_first_last_if_dead'] == true)
            {
                $this->links_html .= '<li><span class="'.$this->options['class_dead_links'].'">'.$this->options['text_last'].'</span></li>'.PHP_EOL;
            }
            elseif($this->current_page != $this->total_pages)
            {
                $this->links_html .= '<li><a class="'.$this->options['class_live_links'].'" href="'.$this->build_link_url($this->total_pages).'">'.$this->options['text_last'].'</a></li>'.PHP_EOL;
            }
        }

         /*
         * Close the UL
         */
        $this->links_html .= '</ul>'.PHP_EOL;
    }


    /**
    * debug(void)
    *
    * Show the debug log
    *
    * @access  public
    * @return  void
    */
    public function debug()
    {
        print_r($debug_log);
    }

}


?>

And here's an example of how it's called:

<?

$options = array(
    'results_per_page'              => 10,
    'url'                           => 'http://www.domain.com/somepage.php?page=*VAR*',
    'db_conn_type'                  => 'pdo',
    'db_handle'                     => $dbh
);

$paginate = new paginate($page, 'SELECT cols FROM table', $options);

$result = $paginate->resultset->fetchAll();

?>

ta!

\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

I'm not going to do a full code review, but there are a couple of issues that I think need to be addressed.

Everything's public in your class, this is very bad because it means that external agents can scribble all over the class internal state. You should definitely make all your properties (variables) protected or private and provide a set of public setters and getters instead, as this will give you more control over what external state consumers of your class can change and how. I've already given a few answers that cover the benefits of getters and setters, so you might want to look those up ;)

When designing a class you should be thinking about what the class embodies, what it's meant to accomplish and what services it's providing to consumers of the class. What is the consumer going to ask the class to do and what output can the consumer expect in return? For these services you need to provide a public interface (public methods/functions) so consumers can ask the class to perform some service for them and collect the results. Anything else that the class does internally to achieve the goal of providing the service it implements should not be publicly available to consumers because consumers don't need to know about how a class does what it does, only that the class provides that service. The more of the internals of your class you expose to outside agents, the harder it becomes to make changes without breaking something that depends on the class.

Your constructor is too big. Constructors should do nothing more than initialize the class to a usable state, they shouldn't do any actual work, because if you want to subclass a class to give it different behaviour and a lot of behaviour is defined in the constructor then you will need to either inherit a lot of behaviour you don't want, or rewrite the constructor to completely override what the superclass constructor does, possibly resulting in a lot of duplication of effort as you rewrite the bits of the superclass constructor that you do want. It also means that you have less opportunity to configure an instance of a class before asking it do provide its service for you.

Your build_links method is also too big. This means it's inflexible and difficult to modify without causing other issues elsewhere. If you split the method down into smaller chunks, then you can more easily swap those chunks out for different ones should you choose to subclass your class, thus making it easier to adapt your class to work in different ways. For example, all the code between each of the function's first level of if statements (the ones with the least indentation) could be split out into their own (protected) methods. This will make the main method shorter, and the methods in question can be easily overridden in any subclasses you choose to make. Also, if you notice you have several methods doing similar work then you have an opportunity to come up with a way to generalize the operation being done and removing some code from your class.

Long methods/functions have other issues too when it comes to understandability and maintainability. A shorter method is easier to understand and therefore maintain, so long methods should be considered a code smell and refactored out. A good rule of thumb is, if you need to scroll to fit the method's body into your editor screen then you probably need to split it into smaller chunks of functionality.

Programming is all about divide and conquer (splitting a big problem into smaller problems and solving each small problem until you have a solution to the big problem they're a part of). Additionally, good code isn't code where there's nothing left to add, but when there's nothing left to take away.

\$\endgroup\$
1
  • \$\begingroup\$ Thanks Gordon, so really good input here :) Also, Code Smell? I like that haha! I think I can split the build links into at least 3 smaller methods anyway so that should be simple enough too. Ta! \$\endgroup\$ Commented Feb 25, 2012 at 12:59

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.