[PHP]poprawność kodu, pisanie klas Opcje

[wpaski]

Prosiłbym o przyjrzenie się tej prostej klasie i ew. wskazanie mi błędów co zmienić w sposobie pisania, zastanawiałem się czy użyć dla zmiennych private czy public, uznałem, że nie ma potrzeby dostępu z poza klasy, druga moja wątpliwość to kod html zwracany w funkcji pobierzZdjecia(), czy należało by go odseparować? Np jeżeli chciałbym użyć tą klasę w stronie opartej o wzorzec MVC, a ta klasa była by w bibliotekach ( czyli tzw. helper?)

[PHP] pobierz, plaintext 
<?php
class Galeria {
 
	private $kat;
	private $katalog = 'obrazki/';
	private $katalogminiaturki = 'miniaturki/';
	private $galeria;
	private $zdjecie;
	private $odczyt;
 
	public function __construct() {
	   $this->kat = isset($_GET['kat']) ? $_GET['kat'] : 'dyrdy';
	   $this->katalog = $this->katalog.$this->kat;
	   $this->katalogminiaturki = $this->katalogminiaturki.$this->kat;   
	}
	public function pobierzZdjecia() {
		$this->galeria = opendir($this->katalog);	
		while($this->zdjecie = readdir($this->galeria)) {
			$this->odczyt = pathinfo($this->katalog.'/'.$this->zdjecie);
				if($this->odczyt['extension'] == 'jpg') {
					return '<li><a href="'.$this->katalog.'/'.$this->zdjecie.'" class="highslide" onclick="return hs.expand(this)" title="Zdjęcie: '.$this->zdjecie.'"><img width="200" height="133" src="'.$this->katalogminiaturki.'/'.$this->zdjecie.'" alt="Zdjęcie: '.$this->zdjecie.'" /></a></li>';
			}
		}
	}	
}
 
$galeria = new Galeria();
echo $galeria->pobierzZdjecia();
 
?>
[PHP] pobierz, plaintext 

[CuteOne]

[PHP] pobierz, plaintext 
//zamien
$this->katalog = $this->katalog.$this->kat;
//na
$this->katalog = $this->kat;
//a to na 
$this->setKatalog($this->kat);
[PHP] pobierz, plaintext 

Zobacz na swój kod - tak naprawdę są to funkcje al'a "Strukturalne PHP" z dodatkiem $this.

1. Podstawowa zasada, której staram się trzymać mówi, że dany obiekt powinien odpowiadać tylko jednej funkcjonalności. U Ciebie jest to mix z czytania plików z katalogu, sprawdzenia extension i zwrócenia html
2. Nawet w pisaniu strukturalnym nie powinno się mieszać HTML z PHP ty tutaj robisz to w sposób brutalny (IMG:style_emoticons/default/wink.gif)
3. Jeżeli sprawdzasz listę plików z katalogu to

[PHP] pobierz, plaintext 
// Tworzymy nową kolekcję dla zdjęć
$this->_imagesCollection = new Collections_ImagesCollection();
 
// Obiekt odpowiedzialny za zarządzanie plikiem
$this->_extension = new Library_File_Manager();
 
while($files = readdir($this->galeria)) {
 
       if($this->_extension->checkExtension($file)) {
 
             $this->_imagesCollection -> addImage(new ImagesEntity($file));
       }
}
[PHP] pobierz, plaintext 

I działamy dopiero na kolekcji zdjęć.

edit: Crozin wpadł na chwilę (IMG:style_emoticons/default/smile.gif) więc pewnie napisze coś mądrzejszego niż moje wypociny (IMG:style_emoticons/default/tongue.gif)

Ten post edytował CuteOne 22.09.2012, 19:37:35