[OOP] Klasa do filtrowania tekstu Opcje

[_olo_1984]

Witam

napisałem taką oto klasę, do filtrowania tekstu. W związku z tym, że dopiero zaczynam zabawę z OOP (w strukturalnym radzę sobie całkiem dobrze i czas podnieść poprzeczkę) mam do Was następujące pytania:
1. Czy klasa którą napisałem to nie pseudo oop? czy tak to powinno wyglądać (czy dobrze kombinuje)
2. Czy filtrowanie danych wchodzących do bazy (tryb 1) jest dobrym zabezpieczeniem przed atakami?
3. Już poza tym tematem, chodzi o smarty (z których tez korzystam), jak w innym obiekcie moge urzyć smarty, oraz jak w pliku inkludowanym urzyć smarty np. mam w głównym katalogu index.php (tu smarty działa ok), ale dołanczam do indexa plik z katalogu /includes/1.php i w tym pliku 1.php gdy prubuję uzyc szablonu, to nie działa (nie dolancza plikow tpl). smarty dolanczam w index. a po nim ten plik 1.php (mam nadzieje ze nie zamieszalem).

poniżej kod:

[PHP] pobierz, plaintext 
<?php
 
class text {
	private $zakazane = array("script", "SCRIPT",
					   "type", "TYPE",
					   "name", "NAME",
					   "union", "UNION",
					   "select", "SELECT",
					   "delete", "DELETE",
					   "modify", "MODIFY",
					   "show", "SHOW",
					   "NULL", "null", "NUL", "nul",
					   "<", ">", "'", "/", " ", "&", "%", ";", ":", "*", "&", "?");
	public $tekst;
	public $zmieniony;
	public function filter($tryb) {
		switch($tryb) {
			case '1':
			return $this->zmien_tekst_1();
			break;
 
			case '2':
			return $this->zmien_tekst_2();
			break;	
 
			case '3':
			return $this->zmien_tekst_3();
			break;		
		}
	}
	private function show_tekst() {
		echo $this->zmieniony;
	}
	private function zmien_tekst_1() {
		$this->zmieniony=str_replace($this->zakazane, "", strip_tags($this->tekst));
		return $this->show_tekst();
	}
	private function zmien_tekst_2() {
		$this->zmieniony=stripslashes($this->tekst);
		return $this->show_tekst();
	}
	private function zmien_tekst_3() {
		$this->zmieniony=htmlentities($this->tekst);
		return $this->show_tekst();
	}
}
$text=new text;
?>
[PHP] pobierz, plaintext 

pozdrawiam

[Cysiaczek]

Mi sie nie podoba kilka rzeczy, w tym jedna jest błędem.

1. Stosuj angielskie nazwy zmiennych, metod itd. Polskie wyglądają po prostu komicznie, a u Ciebie to wogóle jest esperanto
2. W show_text() widze jakieś echo. Błąd! nigdy echo w takiej metodzie. Zawsze return
3. Nazwy metod typu zmien_tekst_1() nc nie mówią o tym, co dana operacja robi. Jedną z zasad OOP jest nazywanie metod zgodnie tym, co robią. Zwłaszcza, jeśli dotyczy to interfejsu publicznego (wiem, masz prywatny, ale czytaj dalej)
4. To, że zastosuję filter($tryb) też mi nic nie mówi

[PHP] pobierz, plaintext 
<?php
$text=new text();
$text->tekst="<b>lol</b>";
$text->filter(2); // No i skąd mam widzieć, co robi filter z parametrem 2?
?>
[PHP] pobierz, plaintext 

5. Ograniczasz funkcję strip_tags() - ona ma jeszcze więcej parametrów, których teraz nie można użyć.

Podsumowując. Kod jest raczej bezużyteczny, bo powoduje więcej problemów niż rozwiązuje i do tego jest nieintuicyjny.
Nie martw się jednak, bo akurat tego typu klasy zazwyczaj są mało przydatne. Zazwyczaj taki kod leci automatycznie gdzieś we framworku, albo wogóle w modelu. To nie jest najlepszy pomysł na naukę oop - lepiej postaraj się wymodelować jakieś zachowania programu np. klasę News, albo księgę gości etc.

Pozdrawiam

[cinekz]

1. Skoro mówimy już o OOP to ta klasa nie ma z nim nic wspólnego. To powinno wyglądać inaczej. Powinieneś zrobić klasę bazową dla każdego filtra. Każdy filtr byłby osobną klasą wyprowadzoną z klasy głównej. Przykładowy kod:

[PHP] pobierz, plaintext 
<?php
abstract class Filter //klasa abstrakcyjna dla wszystkich filtrów
{
	private $value = '';
 
	public function __construct( $value = null )
	{
		$this->setValue( $value );
		$this->run();
	}
 
	public function setValue( $value )
	{
		if( !is_null( $value ) )
		{
			$this->value = $value;
		}
	}
 
	public function getValue()
	{
		return $this->value;
	}
 
	abstract protected function run();
}
 
class HtmlEntitiesFilter extends Filter // implementacja Filtra zamieniające tagi na encje
{
	protected function run()
	{
		$this->setValue( htmlentities( $this->getValue() ) );
	}
}
 
class AddSlashesFilter extends Filter // przykład Filtra dodającego slashe
{
	protected function run()
	{
		$this->setValue( addslashes( $this->getValue() ) );
	}
}
 
 
$filter = new HtmlEntitiesFilter( '<b>sasasas</b>' );
 
echo $filter->getValue(); // wynik:  &lt;b&gt;sasasas&lt;/b&gt;
?>
[PHP] pobierz, plaintext 

Myślę, że pomogłem. Pozdrawiam Marcin.

PS Dąż raczej do prywatności składowych klasy i udostępniaj akcesory.

Ten post edytował cinekz 26.02.2008, 16:26:34

[_olo_1984]

aha ...

dzięki za odpowiedzi panowie. Bede sie starał.


Pozdrawiam

[Kocurro]

a jak napiszę DeLeTe to filtr się wy....sypie (IMG:http://forum.php.pl/style_emoticons/default/winksmiley.jpg)

pozdr.

[Sedziwoj]

@cinekz
Tak z ciekawości po co set/get?
Przy Twoim pomyśle organizacji wystarczy przecież metoda np. filter() która pobiera wartość i ją od razu zawraca, bo jak stosujesz filtr to najczęściej od razu chcesz mieć dane.
Do tego przekazywanie przez konstruktor, jest dziwne, ogólnie to raczej powinny być statyczne metody, bo skoro nie konfigurujemy takich filtrów dynamiczne tworzenie obiektów jest zbędne.

No i w przykładzie jest błąd, bo setValue() też powinna $this->run(), aby nowa wartość była przefiltrowana i jak null, to przypisz null, a nie nic nie rób, bo ktoś przypisze null, a dostanie w zwrocie poprzednią wartość.

No i jest jak napisał Cysiaczek, tego sie raczej nie pisze bo zawsze jest zaszyte w narzędzia z których się normalnie korzysta. A najlepiej jak są odpowiednio wbudowane w nie, bo to wtedy ułatwia, a taka ogólna klasa to tylko ładnie opakowuje jedynie.

[cinekz]

Kod był pisany na szybko, więc błędy w nim się mogły pojawić, to raz. Dwa, kod był tylko i wyłącznie w celu pokazania idei OOP oraz dziedziczenia, ponieważ autor pytał się czy idzie w dobrą stronę. Też nie jestem zwolennikiem takich rozwiązań, ponieważ po prostu filtruje to co mi potrzeba w danych narzędziach. Jeśli chodzi o set/get i prywatność składowych, to jeżeli stosujemy set to mamy pewność jaki typ znajduje się w składowej oraz nikt niepowołany nie użyje jej w nieodpowiedni sposób.

To taki mój lekki wywód na post Sedziwoj'a, ale myślę, że był potrzebny, aby rozjaśnić sytuację.

Dziękuję za krytykę i pozdrawiam - Marcin.

Ten post edytował cinekz 27.02.2008, 15:22:31

[Sedziwoj]

Jeśli chodzi o set/get i prywatność składowych, to jeżeli stosujemy set to mamy pewność jaki typ znajduje się w składowej oraz nikt niepowołany nie użyje jej w nieodpowiedni sposób.

Chyba nie do końca mnie zrozumiałeś, po prostu moim zdaniem klasa nie musi mieć żadnych właściwości (atrybutów/pól/składowych), bo przetwarza dane na bieżąco.
Czyli metoda która przyjmuje, weryfikuje, filtruje i zwraca przefiltrowane dane (oczywiście można wykorzystać, a nawet lepiej Compose Method... aby sama metoda nie robiła wszystkiego).
Ale dla użytkownika daje to ułatwienie użytkowania.
Bo przecież nie atakowałem idei set/get dla właściwości obiektu, było by to śmieszne skoro jestem tego zwolennikiem (raczej wolę sformalizowane działania)

[cinekz]

Widocznie Cię nie zrozumiałem, sorry.