[PHP] Skrypt rejestracji (OOP) - prośba o poprawę kodu Opcje

[Testosteron]

Witajcie!

Chcę nauczyć się programowania obiektowego. Niestety, kursy internetowe opierają się na nieco głupich przykładach, w stylu:
- zaszczekaj burku
- hau hau
Nie ma to przełożenia na realne problemy z jakimi stykamy się w programowaniu. W mojej książce, "PHP i MySQL. Vademecum profesjonalisty" także programowanie obiektowe nie zostało dobrze opisane. Postanowiłem więc spróbować napisać coś prostego. W moim przypadku był ro system rejestracji. Skrypt działa w 100%, chociaż nie jest dokończony (nie ma walidacji hasła, a walidacja loginu też nie jest w pełni skończona). Chodzi mi raczej o samą zasadę działania. Od razu mówię, że jest to mój PIERWSZY, obiektowy projekt pisany bez żadnego wzoru. Zależy mi na tym, żeby ktoś poradził, co należy zmienić.

[PHP] pobierz, plaintext 
<?php
/*    class connect{
        function __construct(){
        $polaczenie = @new mysqli('localhost', 'root', 'haslo', 'login');
        if (mysqli_connect_errno() != 0){
            echo '<p>Wystąpił błąd połączenia: ' . mysqli_connect_error() . '</p>';
        } 
    }
}
*/
 
    class formularz{
        function wyswietl(){
            echo"<table>
                    <form action='register.php' method='post'>
                        <tr><td>Login:</td><td><input type='text' name='login'></td></tr>
                        <tr><td>Password:</td><td><input type='password' name='pass'></td></tr>
                        <tr><td><input type='submit' value='zarejestruj'></td></tr></form>
                </table>";
        }
 
    }
 
    class walidacja extends formularz{
 
        //dodawanie usera
        function __construct(){
            $mysqli = new mysqli('localhost', 'root', 'haslo', 'login');
            $login = $mysqli->real_escape_string($_POST['login']);
            $pass = $mysqli->real_escape_string($_POST['pass']);
 
            if (empty($login) || empty($pass) ){
                $this->wyswietl();
            } else {
                if ($this->sprawdz_login($login) ){
                    $query = $mysqli->query("INSERT INTO login VALUE('" . $login . "', '" . $pass . "')");
                    if ($query){
                        echo 'user został dodany';
                    } else {
                        echo 'blad';
                    }
                }
 
            }
        }
 
        //Poprawność loginu
        function sprawdz_login($login){
            $login = $_POST['login'];
            if (strlen($login) < 4){
                echo 'login musi miec przynajmniej 4 znaki';
            } else {
                return true;
            }
        }
    }
 
 
 
    $formularz = new walidacja;
 
?>
[PHP] pobierz, plaintext 

Na początku chciałem napisać klasę connect (obecnie jest zawarta w komentarzu), jednak miałem problem z wykorzystaniem jej, tzn. zależy mi, żeby w jednej klasie zawrzeć wszystkie dane dotyczące logowania do mysql i później żebym mógł w innych klasach wykorzystywać wszystkie metody mysqli. Niestety miałem z tym problem. Jeżeli ktoś wie jak to zrobić, to byłbym także wdzięczny za odpowiedź.

pozdrawiam (IMG:style_emoticons/default/smile.gif)

Ten post edytował Testosteron 3.07.2014, 17:21:48

[ctom]

do przemyślenia : co ma wspólnego "class walidacja" z DB ?

pomocny link do Twoich pytań dependency-injection

[SmokAnalog]

Po pierwsze takie małe kosmetyczne sprawy:

• Nazwy klas zwyczajowo powinny zaczynać się od wielkiej litery
• Nazywaj klasy, zmienne i metody po angielsku

Oprócz tego masz logikę dziwną. Klasa walidacja nie rozszerza w logiczny sposób klasy formularz. To powinna być jedna klasa, albo klasa walidacja powinna przyjmować obiekt formularza jako parametr np. w konstruktorze. Ewentualnie na odwrót, ale tak wydaje mi się to bardziej rozsądne, bo klasa formularz ma sens bez walidacji, a walidacja bez formularza już niezbyt.

Fajnie jak pouczysz się wyjątków, bo wyrzucanie błędów w echo to bardzo zły pomysł.

[Testosteron]

[PHP] pobierz, plaintext 
<?php
/*	class connect{
		function __construct(){
		$polaczenie = @new mysqli('localhost', 'root', 'pass', 'login');
		if (mysqli_connect_errno() != 0){
			echo '<p>Wystąpił błąd połączenia: ' . mysqli_connect_error() . '</p>';
		} 
	}
}
*/
 
	class formularz{
 
 
		//dodawanie usera
		function __construct(){
			$mysqli = new mysqli('localhost', 'root', 'pass', 'login');
			$login = $mysqli->real_escape_string($_POST['login']);
			$pass = $mysqli->real_escape_string($_POST['pass']);
 
			if (empty($login) || empty($pass) ){
				$this->wyswietl();
			} else {
				if ($this->sprawdz_login($login) ){
					$query = $mysqli->query("INSERT INTO login VALUE('" . $login . "', '" . $pass . "')");
					if ($query){
						echo 'user został dodany';
					} else {
						echo 'blad';
					}
				}
 
			}
		}
 
		//Poprawność loginu
		function sprawdz_login($login){
			$login = $_POST['login'];
			if (strlen($login) < 4){
				echo 'login musi miec przynajmniej 4 znaki';
			} else {
				return true;
			}
		}
 
		//wyświetlanie formularza
		function wyswietl(){
			echo"<table>
					<form action='6.php' method='post'>
						<tr><td>Login:</td><td><input type='text' name='login'></td></tr>
						<tr><td>Password:</td><td><input type='password' name='pass'></td></tr>
						<tr><td><input type='submit' value='zarejestruj'></td></tr></form>
				</table>";
		}
 
	}
 
 
 
	$formularz = new formularz;
 
?>
[PHP] pobierz, plaintext 

(...) albo klasa walidacja powinna przyjmować obiekt formularza jako parametr np. w konstruktorze

Chodzi o to, żeby utworzyć obiekt klasy formularz i wywołać metodę wyświetl w konstruktorze?

Chyba cały czas myślę proceduralnie. W sumie rano przeczytałem rozdział dotyczący OOP, wykonałem kilka ćwiczeń, a wieczorem napisałem ten skrypt dla treningu. Wiem co to jest dziedziczenie. Często podaje się przykłady w stylu - klasy rower i samochód dziedziczą od klasy pojazd, itp. Nie jest to jednak typowy problem z którym stykamy się w programowaniu. W jakich przypadkach najczęściej korzysta się z dziedziczenia? W tej chwili do głowy przychodzi mi tylko tworzenie bazy danych. Tam faktycznie pewne pola mogą być wspólne i są logicznie połączone (IMG:style_emoticons/default/smile.gif) .

Ten post edytował Testosteron 4.07.2014, 09:05:10

[Turson]

Dalej masz te same błędy:
Nazewnictwo klas
Ogólnie stosuje się camelCase w nazwach metod i dobrym nawykiem są angielskie nazwy.
Echo w metodach.
Samo zapytanie insert powinno być odzielną metodą.
Do obsługi bazy danych, np. nawiązywanie połączenia czy pobranie obiektu bazy napisz osobną klasę ale nie dziedzicz jej, a niech metoda getDb() będzie statyczna.

Często podaje się przykłady w stylu - klasy rower i samochód dziedziczą od klasy pojazd, itp. Nie jest to jednak typowy problem z którym stykamy się w programowaniu.

Przykład może nie z życia wzięty, ale poprawny. Przykład: RegisterController może dziedziczyć głowny kontroler aplikacji.

[Testosteron]

* Nazewnictwo to mały pikuś. Ten skrypt miał być wyłącznie ćwiczeniem.
* Echo użyłem, ponieważ nie uczyłem się jeszcze obsługi wyjątków.
* Co do insert - poprawię to.
* Próbowałem napisać klasę do łączenia z mysql, ale cos mi nie szło, tzn. nie wiedziałem jak korzystać z wbudowanych metoda mysqli. Kod, który próbowałem napisać jest w komentarzu.

Ten post edytował Testosteron 4.07.2014, 09:08:23

[aras785]

Odnośnie ECHO to nigdy nie używaj tego w metodach ponieważ funkcja powinna zwracać jakąś wartość, a co znią później zrobisz to od Ciebie zależy: sprawdzić warunkiem czy też wyświetlić:

Przykład:

[PHP] pobierz, plaintext 
<?php
class Car
{
	private $model;
 
	public function __construct() {
 
	}
	public function setModel($model) {
		$this->model = $model;
	}
	public function getModel() {
		return $this->model;
	}
}
 
 
$car = new Car;
$car->setModel('audi');
//teraz decyduje co chce z wartoscia zrobic
if($car->getModel()=='audi') echo 'Jest OK';else echo 'Jest ŹLE';
 
//wyświetlenie
echo $car->getModel();
[PHP] pobierz, plaintext 

U Ciebie z danymi nic nie możesz zrobić bo metody nic nie zwracają...

Ten post edytował aras785 4.07.2014, 09:32:55

[SmokAnalog]

Dziedziczenie jest wspaniałe i daje mnóstwo korzyści. Jedną z moich ulubionych jest tzw. polimorfizm. Jako że nie lubisz przykładów nieinformatycznych (ja np. wolę takie na psach, truskawkach itd. (IMG:style_emoticons/default/biggrin.gif) ), to spróbuję podać informatyczny. Ba, podam nawet przykład dla klasy formularzy.

Prędzej czy później będziesz chciał zrobić obiekty pól formularza. To jest doskonałe miejsce dla dziedziczenia, bo one będą miały jakieś części wspólne, na przykład:

[PHP] pobierz, plaintext 
class InputText extends InputField {...}
class InputCheckbox extends InputField {...}
// teraz uważaj:
class InputTextarea extends InputText {...}
[PHP] pobierz, plaintext 

Obiekt generujący textarea mógłby dziedziczyć po obiekcie generującym zwykłe pole tekstowe, bo będą miały prawdopodobnie wiele wspólnego, a jedną z niewielu różnic będzie metoda, która wypluwa ich HTML.

W klasie formularza mógłbyś mieć metodę, która dodaje pole na zasadzie:

[PHP] pobierz, plaintext 
$form = new Form;
$textarea = new InputTextarea;
 
$form->add($textarea);
[PHP] pobierz, plaintext 

A definicja metody add w klasie Form mogłaby wyglądać tak:

[PHP] pobierz, plaintext 
public function add(InputField $field) {...}
[PHP] pobierz, plaintext 

Tutaj działa nasz polimorfizm, bo mówisz metodzie add, że może przyjmować dowolne pole. Każda z naszych powyższych klas InputText, InputCheckbox i InputTextarea to instancje klasy InputField, bo każda po niej dziedziczy. InputText i InputCheckbox bezpośrednio, a InputTextarea dziedziczy po InputText, czyli też jest dzieckiem klasy InputField (IMG:style_emoticons/default/smile.gif)

[Testosteron]

Nie wiem, czy dobrze Cię zrozumiałem. Mogę stworzyć klasę form, która odpowiada za zwracanie pól formularza. Jej metoda mogłaby przyjmować parametr dotyczący typu pola, czyli - text, password, itp. Inna metoda tej klasy mogłaby ogólnie tworzyć formularz, a więc, np. ustawiałaby metodę get, bądź post i wskazywała na plik do którego mają zostać przesłane dane. Kolejne klasy dziedziczyłyby z tej klasy i odnosiłyby się do konkretnych typów pól formularza. Zawierałyby np. walidację wprowadzonych przez użytkownika danych i zwracałyby true bądź false

Dodatkowo metoda klasy form sprawdzałaby, czy dane zostały wcześniej przesłane. Zwracałaby wartość true bądź false. Poniżej deklaracji wszystkich klas mógłbym stworzyć instrukcję warunkową, która sprawdzałaby, czy dane zostały przesłane. Jeśli nie (a więc odpowiednia metoda zwróci wartość false), tworzy obiekty typu - login, password, mail, itp. W przeciwnym wypadku następuje sprawdzenie poprawności danych i jeśli wszystko jest dobrze, następuje zapisanie ich w bazie danych. W takim układzie tworząc kolejne formularze na stronie (np. komentarze, wyszukiwarka), mógłbym skorzystać z klas stworzonych wcześniej. Oczywiście musiałbym dodać nowe zapytanie, być może nową klasę weryfikującą poprawność danych, ale pracy byłoby o wiele mniej.

Uważasz, że tak byłoby dobrze?

Ten post edytował Testosteron 4.07.2014, 19:23:54

[pedro84]

@Testosteron - nie. Przeczytałeś w ogóle post oraz listingi podane przez @SmokAnaloga?

Uproszczając: masz jedną klasę, Input, która zawiera wszelkie metody wspólne dla wszystkich typów kontrolek formularzy, np. Input::getName(). Tworzysz sobie x klas, dla każdego typu kontrolek, które chcesz wykorzystać: np. Checkbox, Text, Textarea, które dziedziczą po klasie Input.

[Testosteron]

Czytałem, ale przed samym wyjściem. A metody walidacji wstawić w klasach konkretnych inputów?

[pedro84]

Walidacja moim zdaniem powinna być osobnym bytem. Stosuj zasadę jednej odpowiedzialności (ang. Single responsibility principle).

Za Wikipedią:

Martin utożsamia odpowiedzialność klasy z powodem do jej modyfikacji. Możemy rozważyć moduł, który generuje i drukuje raport. Odpowiada on za dwa procesy, a tym samym mogą wystąpić dwa powody do jego modyfikacji. Po pierwsze, może zmienić się treść generowanego raportu, po drugie ? format, w jakim jest on drukowany. Zasada pojedynczej odpowiedzialności mówi, że oba te procesy powinny być niezależne i zaimplementowane w postaci dwóch oddzielnych klas lub modułów, które komunikują się ze sobą przy pomocy publicznych interfejsów.

Zobacz sobie jak to jest rozwiązane np. w komponencie Validation z Symfony.

Ten post edytował pedro84 5.07.2014, 10:34:03

[Testosteron]

Ściągnąłem Symfony. Muszę przyznać, ze ten kod jest dla mnie jest trochę skomplikowany. Zauważyłem ponadto , że większość klas dziedziczy od sfValidatorRegex. Ona z kolei dziedziczy od sfValidatorString, itp. Jeszcze dobrze nie rozumiem zagadnień związanych z OOP. Myślałem, żeby nieco poduczyć się Javy. Nie tylko nauczę się lepiej programować obiektowo, ale ta wiedza też się nie zmarnuje:

1. Coraz więcej stron posiada mobilne wersje stron.
2. Często strony tworzą aplikacje, które mają za zadania rozreklamować daną stronę.
3. Czasami przydałoby mi się stworzyć aplikację desktopową, itp.

Chciałbym rozwijać się głównie w programowaniu stron WWW (znam też trochę jQuery, a JS musiałbym sobie przypomnieć). Moją największą bolączką w PHP jest właśnie OOP, a wydaje mi się, że Java jest dobrym językiem do nauki obiektówki.

PS. Jak znajdę chwilę czasu to postaram się stworzyć ten skrypt jeszcze raz, według wskazówek, które mi udzieliliście.

[pedro84]

Chodziło mi o Symfony 2.

Najpierw musisz poznać podstawy programowania obiektowego, a dopiero potem poszczególne implementacje w danych językach. PHP nie jest już tak tragiczny w implementacji OOP jak parę lat temu (co nie znaczy, że nie mogłoby być lepiej).

PS. Co ma Java do stron mobilnych?

[Testosteron]

Chodziło mi o aplikacje na urządzenia mobilne. Dużo stron posiada takie aplikacje, które ułatwiają korzystanie z nich, np. FB, nk, Kwejk, Demotywatory, itp.

Natrafiłem na pewien problem w kodzie. Otóż - muszę stworzyć klasę, która sprawdza, czy dane zostały przesłane z formularza (następuje ich weryfikacja) czy nie (wyświetlenie formularza). W poprzednim kodzie sprawdzałem czy którakolwiek ze zmiennych globalnych nie została wysłana. W nowym kodzie mogę stworzyć dowolną ilość kontrolek formularza o dowolnych nazwach. Jak więc zrobić taką klasę? Może po prostu zrobić to już poza klasami?

Faktycznie, ściągnąłem starszą wersję. W sumie za bardzo nie wczytałem się na ich stronie, ale na pierwszy rzut oka widzę, ze jest trochę zabawy z instalacją Symphony 2. Trzeba zainstalować Compozera, itp. W Zend pewnie też są w miarę porządnie napisane klasy walidacji danych. Jutro ściągnę Zenda albo ę Symphony 2, bo dzisiaj jestem już zmęczony.

Trochę poprawiłem kod, ale na razie zrobiłem tylko szkielet.

[PHP] pobierz, plaintext 
<?php
	class form{
		private $name, $value;
 
		public function setName($name){
			$this->name=$name;
		}
		public function getName(){
			return $this->name;
		}
		public function setValue($name){
			$this->value=$value;
		}
		public function getValue(){
			return $this->value;
		}
 
		public function newForm($method, $file){
			echo '<form action="' . $file . '" method="' . $method . '" />';
		}
 
		public function finish(){
			echo '</form>';
		}
	}
 
	class inputText extends form{
		public function createText(){
			return '<input type="text" name="' . $this->getName() . '" />';
		}
	}
 
	class inputPass extends form{
		public function createPass(){
			return '<input type="password" name="' . $this->getName() . '" />';
		}
	}
 
	class inputSubmit extends form{
		public function createSubmit(){
			return '<input type="submit" value="' .$this->getValue(). '" />';
		}
	}
	class validation{
		private $variable, $type;
 
		public function setValidation($variable, $type){
			$this->variable = $variable;
			$this->type = $type;
		}
		public function validateForm(){
			htmlspecialchars($this->variable); //na razie nie łączę się z bazą danych, więc powinno mi to wystarczyć
			switch ($this->type){
				case 1: //nr 1 oznacza login
					{
						if (strlen($this->variable) < 3){
							return false;
						} else {
							return true;
						}
					}
				//case 2:
				//etc
			}
 
		}
	}
 
	class register{
		private $login, $pass;
 
		public function setRegister($login, $pass){
			$this->login = $login;
			$this->pass = $pass;
		}
 
		public function finalRegister(){
			return 'Zarestrowałem konto o nicku ' . $this->login . '. Jego hasło to: ' . $this->pass;
		}
	}
 
if (empty($_POST['login']) || empty($_POST['pass']) ){
	$form = new form;
		$form->newForm('POST', '8.php');
		$login = new inputText;
		$login->setName('login');
		echo $login->createText();
		$pass = new inputPass;
		$pass->setName('pass');
		echo $pass->createPass();
		$submit = new inputSubmit;
		$submit->setValue('Wyslij');
		echo $submit->createSubmit();
	$form->finish();
} else {
	$valid = new validation;
	$valid->setValidation($_POST['login'], 1);
	if ($valid->validateForm() == false){
		echo "Nie moge zarejestrowac tego usera";
	} else {
		$reg = new register;
		$reg->setRegister($_POST['login'], $_POST['pass']);
		echo $reg->finalRegister();
	}
}
?>	
[PHP] pobierz, plaintext 

Nikt już się nie wypowie?

Ten post edytował Testosteron 6.07.2014, 19:10:17

[Pyton_000]

Drobne uwagi co do kodu.
W klasach dziedziczących powinieneś mieć 1 metodę która nazywa się createInput (jego rodzaj definiuje przecież typ obiektu) Nie ma sensu tworzenia oddzielnych metod dla każdej z nich. Dodatkowo setName powinno się odnosić do formularza a nie do input. Nazwę input przekazuj w konstruktorze obiektu i ustawiaj w klasie. Dzięki temu masz każdy obiekt nazwany i nie musisz stosować setName, getName.

Poza tym ja bym zrobił coś na zasadzie:
- Jest sobie obiekt Form. Posiada on kolekcję inputów. coś na zasadzie:

[PHP] pobierz, plaintext 
$form = new Form('FormularzNowościowy');
$form->put(new InputText('nazwa'));
...
if(isset($_POST)) {
   $validateRules = new Validate()->addRule('nazwaInput', array('nazwa_reguly' => 'cos'));
   $form->validate($validateRules);
   ....
}
....
$form->show();
[PHP] pobierz, plaintext 

To tak na szybko.

[golabow]

Może pomyśl o klasie abstrakcyjnej, wydzielisz do niej wszystkie właściwości i wymusisz interfejs metodą abstrakcyjną. Zauważ że wszystkie te klasy własnie robią podobną czynność. Potem wystarczy użyć polimorfizmu i masz ładny kod.

[Testosteron]

Macie rację. Postaram się poprawić kod. Tylko, że klasa form sama w sobie posiada metody, tworzące znacznik <form> (wraz z atrybutami) oraz znacznik zamykający. Jeśli stworzę klasę abstrakcyjną to nie będę mógł stworzyć obiektu tej klasy i korzystać z jej metod w taki sposób jak to robię obecnie.

Ten post edytował Testosteron 17.07.2014, 20:35:15

[pedro84]

A właściwie po co Ci metoda, w której klasa pluje HTMLem, który możesz napisać w widoku?

[Testosteron]

No niby mogę, ale z drugiej strony w widoku można napisać cały formularz. Jeżeli piszę klasę formularza to mam wrażenie, że bez tych dwóch metod byłaby niepełna.