Class not returning variable values

[AdRock]

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'));

 

 

 

 

 

[AdRock]

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;
}

[vinny42]

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.