Witam,

Krótkie mam pytanie, otóż czy dobrze napisałem ten kod? Co tu może być błędnego? Jak powinien wyglądać konstruktor?

Kod PHP:

[PHP] pobierz, plaintext 
<?
class Pagination {
	private $perPage = 0;
	private $countRecords = 0;
	private $page;
	private $from;
	private $startLimit = 0;
	private $val;
	private $min;
	private $max;
	public function __construct($current = null, $limit = null, $startLimit = null)
	{
		$this->page = (int)$current;
		$this->perPage = (int)$limit;
		$this->startLimit = (int)$startLimit;
	}
	private function displayResults($val, $min, $max)
	{
		return ($val >= $min && $val <= $max);
	}
	private function onPage($perPage) {
		return $perPage;
	}
	private function page()
	{
		if(isSet($_GET['page'])) 
		{ 
			$page = $_GET['page'];
		}
		else
		{
			$page += 1;
		}
		return $page;
	}
	private function startLimit()
	{
		$startLimit =  ($this->page()  * $this->onPage(30)) - $this->onPage(30);
		return $startLimit;
	}
	private function dataSelect($from) 
	{
		$row = mysql_fetch_array(mysql_query('SELECT COUNT(*) AS total FROM ' . $from . ';'));
		return $row['total'];
	}
 
 
	private function firstPage() {
		return 1;
	}
	private function lastPage() {
		$allPages = ceil($this->dataSelect('city') / $this->onPage(30));
		return $allPages;
	}
	private function currentPage() {
		return $this->page();
	}
	private function previousPage() {
		if($this->page() > 1)
		{
			return '<a href="' . $_SERVER['PHP_SELF'] . '?page='.($this->currentPage() - 1).'">< poprzednia</a>';
		}
		else
		{
			return false;
		}
	}
	private function nextPage() {
		if($this->page() != $this->lastPage())
		{
			return '<a href="' . $_SERVER['PHP_SELF'] . '?page='.($this->currentPage() + 1).'">nastepna ></a>';
		}
		else
		{
			return false;
		}
	}
	public function allPages() { 
		if($this->page() > 4)
		{
			echo '&nbsp;<a href="' . $_SERVER['PHP_SELF'] . '?page='.$this->firstPage().'"><< Pierwsza </a>';
		}
		echo $this->previousPage();
		for($i = 1; $i <= $this->lastPage(); $i++)
		{
			if($this->displayResults($i, ($this->page() - 3), ($this->page() + 5)))
				if($_GET['page'] == $i)
				{
					echo '<a href="' . $_SERVER['PHP_SELF'] . '?page=' . $i . '"><b>' . $i . '</b>&nbsp;';
				}
				else
				{
					echo '<a href="' . $_SERVER['PHP_SELF'] . '?page=' . $i . '">' . $i . '</b>&nbsp;';
				}
		}
		echo $this->nextPage();
		if($this->lastPage() != $this->currentPage())
		{
			echo '&nbsp;<a href="' . $_SERVER['PHP_SELF'] . '?page='.$this->lastPage().'">ostatnia >></a>';
		} 
		else
		{
			return false;
		}
	}
	public function rowsLimit()
	{
		return  'LIMIT ' .  $this->startLimit() . ', ' . $this->onPage(30) . '';
	}
}
[PHP] pobierz, plaintext 

Praktycznie wszystko jest źle. Uzależniasz się od jednego źródła danych i to jeszcze przestarzały, nieistniejący w PHP7 mysql_*. Linki generowane na sztywno, mieszanie widoku. Zobacz jak to powinno być zrobione choćby na przykładzie https://github.com/zendframework/zend-paginator

Ja mam natomiast pytanie:

[PHP] pobierz, plaintext 
	private function onPage($perPage) {
		return $perPage;
	}
[PHP] pobierz, plaintext 

Po cholere?

MySQL zamiast MySQLi jest tutaj tylko, aby sprawdzić czy to jakkolwiek działa, oczywiście w projekcie będzie użyte MySQLi.

Także, nie chcę gotowca, a wypisać mi błędy jakie mam w kodzie.

Napisałeś, że wszystko jest źle, konkretniej? Paginator jest moim wstępem do OOP.





No i o to chodzi, wytykanie mi błędów.

Sam sobie odpowiedziałeś co jest największym problemem. Chcąc zmienić źródło musisz przepisać całą klasę pod inny sterownik. Nie ma to sensu żadnego. A to samo dotyczy linków i warstwy widoku.

Poczytaj o PSR i trzymaj się standardu formatowania kodu. No i nie mieszaj wszystkich warstw co już zostało napisane