AdRock Posted September 19, 2013 Share Posted September 19, 2013 (edited) I have converted an old pagination function I've had for years to a class as I've recently started to learn OOP The problem is that it doesn't display any anything except the <ul></ul> I've tried returning a variable but it comes back empty. I know it's something to do with my variables but I can't see what class Pagination { private $total_pages; private $page; private $webpage; public function __construct() { $this->total_pages; $this->page; $this->webpage; } /** * This function is called whenever the there are several records to be displayed in the table * This saves the page extending further down the page creating a long list of results * when all the results can be spread across multiple pages */ public function pagination_one($total_pages, $page, $webpage) { // Maximum number of links per page. If exceeded, google style pagination is generated $max_links = 6; $h=1; if($this->page>$max_links) { $h=(($h+$this->page)-$max_links); } if($this->page>=1) { $max_links = $max_links+($this->page-1); } if($max_links>$this->total_pages) { $max_links=$this->total_pages+1; } $paging = ''; $paging .= '<div class="page_numbers"> <ul>'; if($this->page > "1") { $paging .= '<li class="current"><a href="/'.$this->webpage.'/1">First</a></li> <li class="current"><a href="/'.$this->webpage.'/'.($this->page-1).'">Prev</a></li> '; } if($this->total_pages != 1) { for ($i=$h;$i<$max_links;$i++) { if($i==$this->page) { $paging .= '<li><a class="current">'.$i.'</a></li>'; } else { $paging .= '<li><a href="/'.$this->webpage.'/'.$i.'">'.$i.'</a> </li>'; } } } if(($this->page >="1")&&($this->page!=$this->total_pages)) { $paging .= '<li class="current"><a href="/'.$this->webpage.'/'.($this->page+1).'">Next</a></li> <li class="current"><a href="/'.$this->webpage.'/'.$this->total_pages.'">Last</a></li>'; } $paging .= '</ul> </div>'; return $this->page; } } $paging = new Pagination(); var_dump( $paging->pagination_one(3, 1, 'news')); <?php class Pagination { private $total_pages; private $page; private $webpage; public function __construct() { $this->total_pages; $this->page; $this->webpage; } /** * This function is called whenever the there are several records to be displayed in the table * This saves the page extending further down the page creating a long list of results * when all the results can be spread across multiple pages */ public function pagination_one($total_pages, $page, $webpage) { // Maximum number of links per page. If exceeded, google style pagination is generated $max_links = 6; $h=1; if($this->page>$max_links) { $h=(($h+$this->page)-$max_links); } if($this->page>=1) { $max_links = $max_links+($this->page-1); } if($max_links>$this->total_pages) { $max_links=$this->total_pages+1; } $paging = ''; $paging .= '<div class="page_numbers"> <ul>'; if($this->page > "1") { $paging .= '<li class="current"><a href="/'.$this->webpage.'/1">First</a></li> <li class="current"><a href="/'.$this->webpage.'/'.($this->page-1).'">Prev</a></li> '; } if($this->total_pages != 1) { for ($i=$h;$i<$max_links;$i++) { if($i==$this->page) { $paging .= '<li><a class="current">'.$i.'</a></li>'; } else { $paging .= '<li><a href="/'.$this->webpage.'/'.$i.'">'.$i.'</a> </li>'; } } } if(($this->page >="1")&&($this->page!=$this->total_pages)) { $paging .= '<li class="current"><a href="/'.$this->webpage.'/'.($this->page+1).'">Next</a></li> <li class="current"><a href="/'.$this->webpage.'/'.$this->total_pages.'">Last</a></li>'; } $paging .= '</ul> </div>'; return $this->page; } } $paging = new Pagination(); var_dump( $paging->pagination_one(3, 1, 'news')); Edited September 19, 2013 by AdRock Quote Link to comment Share on other sites More sharing options...
Solution AdRock Posted September 19, 2013 Author Solution Share Posted September 19, 2013 Sorted it. I had to remove the private variables and change the constructor to this public function __construct($total_pages, $page, $webpage) { $this->total_pages = $total_pages; $this->page = $page; $this->webpage = $webpage; } Quote Link to comment Share on other sites More sharing options...
vinny42 Posted September 19, 2013 Share Posted September 19, 2013 Uhm no, you very much did *not* have to remove the private properties, they must be in there because those are what you are filling in the construnctor. I would also suggest that you cut the single method up into managable (and testable) submethods (private methods) that do a single and simple task each, like calculate how many links to display, calculate what the next and previous linkst are etc. The whol business the the $h should never be where it is now, it makes the code difficult to follow and therefore bug-sensitive. Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.