Pomoc - Szukaj - Użytkownicy - Kalendarz
Pełna wersja: Optymalizacja klasy do obsługi użytkownika
Forum PHP.pl > Forum > PHP
rav1989
Witam

Napisałem klasę która ma obsługiwać użytkownika wydaje mi się, że jest ona nieoptymalna... wstawiam ją aby ktoś z większym doświadczeniem ją obejrzał i mnie nakierował co można zrobić lepiej i co mam poprawić biggrin.gif

Klasa:
  1.  
  2. define("MySql_Host", "localhost");
  3. define("MySql_Login", "root");
  4. define("MySql_Password", "");
  5. define("MySql_DbName", "test");
  6. define("MySql_TPrefix", "");
  7.  
  8. define("MODE_USER",0);
  9. define("MODE_MOD",1);
  10. define("MODE_ADMIN",2);
  11.  
  12. function removeSlashes(&$value)
  13. {
  14. if(is_array($value))
  15. {
  16. return array_map('removeSlashes', $value);
  17. }
  18. else
  19. {
  20. return stripslashes($value);
  21. }
  22. }
  23.  
  24. class User{
  25. private $id;
  26. private $email;
  27. private $haslo;
  28.  
  29. public $nazwa;
  30. public $opis;
  31. public $avatar;
  32. public $mode;
  33. public $urls;
  34. private $sesja;
  35.  
  36. private $pdo;
  37.  
  38. public function __construct($email_ = null, $haslo_ = null){
  39. $this->pdo = new PDO('mysql:host='.MySql_Host.';dbname='.MySql_DbName, MySql_Login , MySql_Password);
  40. $this->pdo -> setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
  41.  
  42. if(isset($email_) and isset($haslo_)){
  43. $this->email = $email_;
  44. $this->haslo = md5(sha1($haslo_).sha1('TuJestSol'));
  45. }elseif(isset($_SESSION['token'])){
  46. $this->sesja = $_SESSION['token'];
  47. $this->Load();
  48. }else die('Ups! Odwiedź rejestrację!');
  49. }
  50.  
  51. private function mysql_insert($table, $inserts) {
  52. $values = array_map("removeSlashes", array_values($inserts));
  53. $keys = array_keys($inserts);
  54.  
  55. $query = 'INSERT INTO `'.MySql_TPrefix.$table.'` (`'.implode('`,`', $keys).'`) VALUES (\''.implode('\',\'', $values).'\')';
  56. try{
  57. return $this->pdo -> exec($query);
  58. }
  59. catch(PDOException $e)
  60. {
  61. echo 'Wystąpił błąd biblioteki PDO: ' . $e->getMessage();
  62. return false;
  63. }
  64. }
  65.  
  66. public function Rejestruj($nazwa_){
  67. $this->nazwa = $nazwa_;
  68. $this->mode = MODE_USER;
  69. $stmt = $this->pdo -> query('SELECT * FROM `users` WHERE `haslo`="'.$this->haslo.'" and `email`="'.$this->email.'"');
  70. if($stmt->fetch()) return false;
  71. $tmp = $this->mysql_insert('users',array('nazwa'=>$this->nazwa, 'haslo'=>$this->haslo, 'email'=>$this->email, 'mode'=>$this->mode));
  72.  
  73. if($tmp)
  74. return true;
  75. return false;
  76. }
  77.  
  78. public function Loguj(){
  79. try
  80. {
  81. $_SESSION['token'] = md5(rand());
  82. $this->sesja = $_SESSION['token'];
  83. $this->pdo -> query('UPDATE `users` SET `sesja`="'.$_SESSION['token'].'" WHERE `haslo`="'.$this->haslo.'" and `email`="'.$this->email.'"');
  84. }
  85. catch(PDOException $e)
  86. {
  87. echo 'Połączenie nie mogło zostać utworzone: ' . $e->getMessage();
  88. return null;
  89. }
  90. }
  91. public function Wyloguj(){
  92. try
  93. {
  94. $this->pdo -> query('UPDATE `users` SET `sesja`="NULL" WHERE `sesja`="'.$this->sesja.'"');
  95. }
  96. catch(PDOException $e)
  97. {
  98. echo 'Połączenie nie mogło zostać utworzone: ' . $e->getMessage();
  99. return null;
  100. }
  101. }
  102.  
  103. public function __set($name,$value){
  104. $this->$name($value);
  105. }
  106. private function Load(){
  107. try
  108. {
  109. $stmt = $this->pdo -> query('SELECT * FROM `users` WHERE `sesja`="'.$this->sesja.'"');
  110. $row = $stmt->fetch();
  111. $stmt -> closeCursor();
  112. $urls = $this->pdo -> query('SELECT url FROM `user_sites` WHERE `users_id`="'.$row['id'].'"');
  113. foreach($urls as $url){
  114. $this->urls[] = base64_decode($url['url']);
  115. }
  116. $urls -> closeCursor();
  117. $this->opis = $row['opis'];
  118. $this->avatar = $row['avatar'];
  119. $this->nazwa = $row['nazwa'];
  120. $this->mode = $row['mode'];
  121. $this->id = $row['id'];
  122. }
  123. catch(PDOException $e)
  124. {
  125. echo 'Połączenie nie mogło zostać utworzone: ' . $e->getMessage();
  126. return null;
  127. }
  128. }
  129.  
  130. public function AddUrl($url){
  131. $url = base64_encode($url);
  132. $stmt = $this->pdo -> query('SELECT * FROM `user_sites` WHERE `url`="'.$url.'" and `users_id`="'.$this->id.'"');
  133. if($stmt->fetch()) return false;
  134. $tmp = $this->mysql_insert('user_sites',array('url'=>$url, 'users_id'=>$this->id));
  135. $this->urls[] = base64_decode($url);
  136.  
  137. if($tmp)
  138. return true;
  139. return false;
  140. }
  141.  
  142. public function Opis($value){
  143. $this->opis = $value;
  144. try
  145. {
  146. $this->pdo -> query('UPDATE `users` SET `opis`="'.$this->opis.'" WHERE `sesja`="'.$this->sesja.'"');
  147. }
  148. catch(PDOException $e)
  149. {
  150. echo 'Połączenie nie mogło zostać utworzone: ' . $e->getMessage();
  151. return null;
  152. }
  153. }
  154.  
  155. public function Avatar($value){
  156. $this->avatar = $value;
  157. try
  158. {
  159. $this->pdo -> query('UPDATE `users` SET `avatar`="'.$this->avatar.'" WHERE `sesja`="'.$this->sesja.'"');
  160. }
  161. catch(PDOException $e)
  162. {
  163. echo 'Połączenie nie mogło zostać utworzone: ' . $e->getMessage();
  164. return null;
  165. }
  166. }
  167.  
  168. public function Mode($value){
  169. $this->mode = $value;
  170. try
  171. {
  172. $this->pdo -> query('UPDATE `users` SET `mode`="'.$this->mode.'" WHERE `sesja`="'.$this->sesja.'"');
  173. }
  174. catch(PDOException $e)
  175. {
  176. echo 'Połączenie nie mogło zostać utworzone: ' . $e->getMessage();
  177. return null;
  178. }
  179. }
  180.  
  181.  
  182.  
  183. }


Logowanie:
  1. require_once('class/user.class.php');
  2. $user = new User('test','test');
  3. $user -> Loguj();


Update/Odczyt danych:
  1. require_once('class/user.class.php');
  2. $user = new User();
  3. $user->Opis = 'adad';
  4. $user->AddUrl ='http://wp.pl';
  5. echo $user -> opis;
  6. print_r($user -> urls);


Wylogowanie:
  1. require_once('class/user.class.php');
  2. $user = new User();
  3. $user->Wyloguj();


Struktura SQL:
  1. CREATE TABLE IF NOT EXISTS `users` (
  2. `id` INT NOT NULL AUTO_INCREMENT ,
  3. `nazwa` VARCHAR(45) NULL ,
  4. `email` VARCHAR(45) NULL ,
  5. `haslo` VARCHAR(45) NULL ,
  6. `opis` TEXT NULL ,
  7. `avatar` VARCHAR(128) NULL ,
  8. `mode` INT NULL ,
  9. `sesja` VARCHAR(45) NULL ,
  10. PRIMARY KEY (`id`) )
  11. ENGINE = MyISAM
  12. DEFAULT CHARACTER SET = utf8
  13. COLLATE = utf8_general_ci
  14. PACK_KEYS = DEFAULT;

Pozdrawiam
Rav
Valker
Może nie jestem jakimś profesjonalistą, ale zmienił bym parę rzeczy:

1. Wyświetlanie błędu mysql w przypadku braku połączenia jest bez sensu. Nie lepiej po prostu zostawić: "Błąd połączenia z bazą danych".
2. Używanie die() w środku klasy (szczególnie w konstruktorze) też nie jest według mnie najlepszym błędem. Jak dla mnie powinien być zwracany jakiś kod błędu i dopiero wtedy pokazywany błąd. Poza tym korzystanie z die() powoduje przerwanie wykonywanie skryptu. (o ile nie jest to biała strona bez html, dużo znaczników html nie zostanie zakończonych)
3. Tworzenie instancji klasy za każdym razem jak chcesz wykonać jakieś operacje na koncie użytkownika też trochę (jak dla mnie) jest źle pomyślane. Za każdym razem jak dokonujesz zmian jest wcześniej wykonywany kod logowania (a przez to jest dużo nie potrzebnych zapytań do bazy). Wiem, że wszyscy mówią, że korzystanie ze wzorcu Signleton jest złe i w ogóle się nie powinno, ale jak dla mnie to lepsze rozwiązanie niż wykonywanie co chwile kodu logowania.
4. Wykonywanie zapytania do bazy przy wylogowywaniu też jest zbędne. Wystarczy, że z sesji usuniesz zmienną token ;D

To chyba wszystko co od razu rzuciło mi się na oczy, pewnie jest jeszcze parę rzeczy, które wymagały by poprawy, ale może niech pomyśli nad tym ktoś bardziej doświadczony ;P

Pozdr,
Valker
rav1989
Cytat(Valker @ 22.04.2011, 01:22:26 ) *
1. Wyświetlanie błędu mysql w przypadku braku połączenia jest bez sensu. Nie lepiej po prostu zostawić: "Błąd połączenia z bazą danych".
2. Używanie die() w środku klasy (szczególnie w konstruktorze) też nie jest według mnie najlepszym błędem. Jak dla mnie powinien być zwracany jakiś kod błędu i dopiero wtedy pokazywany błąd. Poza tym korzystanie z die() powoduje przerwanie wykonywanie skryptu. (o ile nie jest to biała strona bez html, dużo znaczników html nie zostanie zakończonych)
3. Tworzenie instancji klasy za każdym razem jak chcesz wykonać jakieś operacje na koncie użytkownika też trochę (jak dla mnie) jest źle pomyślane. Za każdym razem jak dokonujesz zmian jest wcześniej wykonywany kod logowania (a przez to jest dużo nie potrzebnych zapytań do bazy). Wiem, że wszyscy mówią, że korzystanie ze wzorcu Cjest złe i w ogóle się nie powinno, ale jak dla mnie to lepsze rozwiązanie niż wykonywanie co chwile kodu logowania.
4. Wykonywanie zapytania do bazy przy wylogowywaniu też jest zbędne. Wystarczy, że z sesji usuniesz zmienną token ;D


1. To jest na razie do testów, później dam funkcję zapisującą logi, oraz będzie się pojawiał stosowny komunikat na stronie biggrin.gif
2. Faktycznie, trochę nie pomyślałem... myślisz, że jak dam przekierowanie za pomocą header do strony rejestracji to będzie ok??
3. Hmm... nie słyszałem to wzorcu Signleton więc go nie zastosowałem, natomiast przed chwilą wlazłem z ciekawości na http://phpedia.pl/wiki/Singleton i poczytałem, bardzo ciekawe to jest, tylko jak ja będę miał wszystko w osobnych plikach to nie za bardzo potrafię sobie wyobrazić jak to będzie działać...
4. Faktycznie :F jedno zapytanie do bazy mniej....

Dziękuję za wskazanie tych błędów, zmiany zostaną wprowadzone biggrin.gif natomiast ja czekam na dalsze propozycję optymalizacji...

Pozdrawiam
Rav
misiek172
można wiele zmienić taka troszkę średnia ta klasa tongue.gif

nie wczytywałem się ale na 1 rzut oka, chociażby rejestracja, po prostu ustaw sobie w bazie pola na unique i jak będzie taki istniał to zwróci błąd
rav1989
Cytat(misiek172 @ 22.04.2011, 12:45:21 ) *
można wiele zmienić taka troszkę średnia ta klasa tongue.gif

nie wczytywałem się ale na 1 rzut oka, chociażby rejestracja, po prostu ustaw sobie w bazie pola na unique i jak będzie taki istniał to zwróci błąd


Rozumiem biggrin.gif ustawiłem email, nazwa oraz sesja na unique...

Jak klasa jest średnia to może mógłbyś nakierować co można jeszcze poprawić... dopiero się uczę pisać klasy w PHP i każda wskazówka jest cenna biggrin.gif
Dipter
1. Definiujesz dane do bazy danych, które równie dobrze mógłbyś wpisać do klasy poprzez choćby tablicę z konfiguracją.
2. Klasa jest kompletnie nie tyle co źle, a bezsensownie napisana.
3. Pakujesz wszystko do jednego - Połączenie z bazą danych, operacje na niej, dane użytkownika, autoryzacja, wyświetlanie.
4. Łączysz się za każdym razem przy próbie utworzenia obiektu User.
5. Prawie w każdej metodzie wykonujesz zapytanie o_O.
6. Wspomniałem wyżej, że klasa jest napisana bezsensu, ponieważ niektóre "metody" zawarte w niej są na siłę wepchane (czyt. Twoja klasa to kontener na funkcję)
7. Podstawy OOP się kłaniają - Single Responsibility Principle - Jedna sprawa - Jedna klasa.

Przede wszystkim musisz oddzielić od siebie taki rzeczy jak baza danych (dostęp, komunikacja, obsługa), zarządzanie użytkownikiem, autoryzacja.

Stwórz klasę, która zajmie się połączeniem z bazą danych, zwróci Ci obiekt jakiegoś sterownika bazy danych (np. klasę dziedziczącą po PDO).

Zrób osobną klasę dla obsługi użytkownika (tworzenie, usuwanie, edycja / pobieranie danych) oraz osobną do autoryzacji (logowanie, wylogowywanie, sprawdzanie).

pozdrawiam wink.gif
rav1989
blink.gif Nie wiedziałem, że jest aż tak źle... no cóż koniec gadania trza brać się do roboty przynajmniej się czegoś człowiek nauczy graduated.gif i będzie mniej błędów popełniał...

Dziękuję Ci za te wskazówki smile.gif

Gdyby ktoś jeszcze wykrył jakieś nieścisłości to dajcie znać biggrin.gif

Pozdrawiam
Rav

EDIT:

Trochę się napisałem (wzorując się na innej klasie) i stworzyłem to:

Klasa pomocnicza
http://pastebin.com/DrBKxin0

Klasa Sesja
http://pastebin.com/XdtQgZ5c

Klasa User
http://pastebin.com/hd6snAJT

Strona bez zabezpieczeń
http://pastebin.com/H2iXn0ki

Logowanie/Wylogowanie
http://pastebin.com/4wEwAH2F

Rejestracja
http://pastebin.com/Na6L10Gm

Strona z zabezpieczeniem
http://pastebin.com/JarmyA5K

Pozdrawiam
Rav
konole
Postaraj się trzymać stałego nazewnictwa. Nazwy funkcji zaczynaj od małej litery, nie wielkiej (klasa User: Rejestruj itd.)
rav1989
Cytat(konole @ 25.04.2011, 22:58:56 ) *
Postaraj się trzymać stałego nazewnictwa. Nazwy funkcji zaczynaj od małej litery, nie wielkiej (klasa User: Rejestruj itd.)


OK, już to poprawiłem biggrin.gif

Jak mam coś jeszcze poprawić to pisać śmiało smile.gif

Pozdrawiam
Rav
Dipter
1. Klasa pomocnicza - W konstruktorze klasy ustalasz na sztywno dane (przeglądarkę i ip), a co jeśli będziesz chciał przekazać to z poziomu aplikacji? Bezsensowne grzebanie w kodzie, po prostu przekaż przy tworzeniu obiektu HttpRequest te dane jako argumenty w konstruktorze, bądź jakichkolwiek metodach. Tak poza tym HttpRequest nie powinien być tylko ustalaczem i pobieraczem do browsera i ip smile.gif
2. Zmienne globalne w klasie, tutaj jest to bardzo niemile widziane. Dlaczego nie przekażesz w dowolnej metodzie/konstruktorze klasy j/w obiektu PDO? Jest też lepsze rozwiązanie stworzenie klasy Singletona, która zwróci Ci jedną instancję obiektu PDO i przy okazji wykona połączenie 1 raz.
3. Co do definiowania takich rzeczy jak długość życia sesji, czy też jej nazwy - są od takich stałe w klasie.
4. Trzymaj się ustalonych zasad nazewnictwa klas/metod/zmiennych - i nie mówię dużych/małych literach z kreskami czy nie, ale też o tym, że getOpis() czy getNazwa() są po prostu śmieszne i pokazują niski poziom autora kodu i nie tylko smile.gif Ustal z góry jakieś zasady - Angielski, to Angielski - Polski, to Polski.
5. Znów wszystko jest namieszane jak leci. Musisz przemyśleć dokładnie jak to ma wyglądać.

Nie wiem, przypatrz się na jakiś projekt oparty o OOP jak mniej więcej to wygląda,
pozdrawiam tongue.gif
rav1989
Cytat
1. Klasa pomocnicza - W konstruktorze klasy ustalasz na sztywno dane (przeglądarkę i ip), a co jeśli będziesz chciał przekazać to z poziomu aplikacji? Bezsensowne grzebanie w kodzie, po prostu przekaż przy tworzeniu obiektu HttpRequest te dane jako argumenty w konstruktorze, bądź jakichkolwiek metodach. Tak poza tym HttpRequest nie powinien być tylko ustalaczem i pobieraczem do browsera i ip smile.gif

Ok biggrin.gif Dodałem przechowywanie POST oraz GET

Tak to teraz wygląda
  1. class httpRequest
  2. {
  3. private $ip;
  4. private $browser;
  5. private $post;
  6. private $get;
  7.  
  8. public function __set($name,$value)
  9. {
  10. $this -> $name = $value;
  11. }
  12.  
  13. public function sendGet($array)
  14. {
  15. $_GET = $array;
  16. }
  17.  
  18. public function sendPost($array)
  19. {
  20. $_POST = $array;
  21. }
  22.  
  23. public function getGet($key = null)
  24. {
  25. return ($key == null ? $this->get : $this->get[$key]);
  26. }
  27.  
  28. public function getPost($key = null)
  29. {
  30. return ($key == null ? $this->post : $this->post[$key]);
  31. }
  32.  
  33. public function getIp()
  34. {
  35. return $this -> ip;
  36. }
  37.  
  38. public function getBrowser()
  39. {
  40. return $this -> browser;
  41. }
  42. }


A tak się inicjalizuje
  1. $request = new httpRequest;
  2.  
  3. $request -> ip = $_SERVER['REMOTE_ADDR'];
  4. $request -> browser = $_SERVER['HTTP_USER_AGENT'];
  5. $request -> get = $_GET;
  6. $request -> post = $_POST;


Cytat
2. Zmienne globalne w klasie, tutaj jest to bardzo niemile widziane. Dlaczego nie przekażesz w dowolnej metodzie/konstruktorze klasy j/w obiektu PDO? Jest też lepsze rozwiązanie stworzenie klasy Singletona, która zwróci Ci jedną instancję obiektu PDO i przy okazji wykona połączenie 1 raz.


Nie wiem czy zrozumiałem ten wzorzec Singleton... zrobiłem tak:
  1. class DB extends PDO
  2. {
  3. private static $instance;
  4. private function __clone(){}
  5.  
  6. public static function getInstance ($dsn, $username = null, $password = null, $driver_options = null)
  7. {
  8. if (self::$instance === null) {
  9. self::$instance = new self($dsn, $username, $password, $driver_options);
  10. self::$instance -> setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
  11. }
  12. return self::$instance;
  13. }
  14. }

  1. $pdo = DB::getInstance('mysql:host='.MySql_Host.';dbname='.MySql_DbName, MySql_Login , MySql_Password);


Pozbyłem się też tych zmiennych globalnych i przekazuję wszystko przy pomocy konstruktora ew. jako zmienna w metodach statycznych biggrin.gif

Cytat
3. Co do definiowania takich rzeczy jak długość życia sesji, czy też jej nazwy - są od takich stałe w klasie.

Poprawiłem i przeniosłem jako zmienne prywatne do klasy biggrin.gif

Cytat
4. Trzymaj się ustalonych zasad nazewnictwa klas/metod/zmiennych - i nie mówię dużych/małych literach z kreskami czy nie, ale też o tym, że getOpis() czy getNazwa() są po prostu śmieszne i pokazują niski poziom autora kodu i nie tylko smile.gif Ustal z góry jakieś zasady - Angielski, to Angielski - Polski, to Polski.

OK. Pozmieniałem wszystkie nazwy Polskie na Angielskie odpowiedniki (również w bazie danych)

Cytat
5. Znów wszystko jest namieszane jak leci. Musisz przemyśleć dokładnie jak to ma wyglądać.

Nie rozumiem... dlaczego jest namieszane?

Pozdrawiam
Rav
Dipter
Cytat
Cytat
5. Znów wszystko jest namieszane jak leci. Musisz przemyśleć dokładnie jak to ma wyglądać.

Nie rozumiem... dlaczego jest namieszane?


Staraj się by jedno zajmowało się jednym, a drugie drugim.

1. HttpRequest - Dodałeś GET i POST, ale co z SERVER, REQUEST, SESSION, COOKIE etc. Poza tym nie lepiej by była to klasa statyczna, bądź Singleton by nie przekazywać non stop obiektu?
2. Klasa DB - Dobry pierwszy krok, ale Singleton z tego marny, właściwie w ogóle go brak tongue.gif getInstance() powinien zwracać Ci obiekt klasy DB, oraz przy okazji klasy PDO.

Przykład:

  1.  
  2. class DB extends PDO {
  3.  
  4. static private $instance = null;
  5.  
  6. public function __construct() {}
  7.  
  8. static public function getInstance() {
  9. if(self::$instance === null) {
  10. self::$instance = new self();
  11. }
  12. return self::$instance;
  13. }
  14.  
  15. static public function connect($config = array()) {
  16. parent::__construct($config['dsn'], $config['username'], $config['password'], [...]);
  17. }
  18.  
  19. }
  20.  


I wykorzystanie na początku aplikacji?

  1.  
  2. DB::connect(array([...]));
  3.  
  4. DB::getInstance()->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
  5.  


3. Co do klasy User - To tutaj głównie mam zarzuty co do mieszania. Masz tutaj rejestracje, sprawdzanie, pobieranie informacji o użytkowniku oraz zwracanie czy to Admin, czy Moderator. Jeśli mowa tutaj o optymalizacji trzeba też wspomnieć o jakimś ładzie - Osobna klasa do rejestracji użytkownika, sprawdzania czy istnieje i innych. Inna znowu do pobierania danych użytkownika o id / nazwie x, oraz ew. rozszerzenie do klasy pobierającej dane, która ustali czy to Admin/Mod/Użytkownik czy ktokolwiek.

Życzę miłej pracy tongue.gif
rav1989
Cytat
1. HttpRequest - Dodałeś GET i POST, ale co z SERVER, REQUEST, SESSION, COOKIE etc. Poza tym nie lepiej by była to klasa statyczna, bądź Singleton by nie przekazywać non stop obiektu?

Nie mam pojęcia jak to zrobić aby było to Singleton... natomiast dodałem SERVER i COOKIE (jakoś reszty nie potrzebuję) analogicznie jak zrobiłem to z GET i POST

Cytat
2. Klasa DB - Dobry pierwszy krok, ale Singleton z tego marny, właściwie w ogóle go brak tongue.gif getInstance() powinien zwracać Ci obiekt klasy DB, oraz przy okazji klasy PDO.

Przykład:

  1.  
  2. class DB extends PDO {
  3.  
  4. static private $instance = null;
  5.  
  6. public function __construct() {}
  7.  
  8. static public function getInstance() {
  9. if(self::$instance === null) {
  10. self::$instance = new self();
  11. }
  12. return self::$instance;
  13. }
  14.  
  15. static public function connect($config = array()) {
  16. parent::__construct($config['dsn'], $config['username'], $config['password'], [...]);
  17. }
  18.  
  19. }
  20.  


I wykorzystanie na początku aplikacji?

  1.  
  2. DB::connect(array([...]));
  3.  
  4. DB::getInstance()->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
  5.  

Hmm...
Kod
Fatal error: Non-static method PDO::__construct() cannot be called statically in C:\wamp\www\class\db.class.php on line 33
nie wiem co poszło nie tak...


Cytat
3. Co do klasy User - To tutaj głównie mam zarzuty co do mieszania. Masz tutaj rejestracje, sprawdzanie, pobieranie informacji o użytkowniku oraz zwracanie czy to Admin, czy Moderator. Jeśli mowa tutaj o optymalizacji trzeba też wspomnieć o jakimś ładzie - Osobna klasa do rejestracji użytkownika, sprawdzania czy istnieje i innych. Inna znowu do pobierania danych użytkownika o id / nazwie x, oraz ew. rozszerzenie do klasy pobierającej dane, która ustali czy to Admin/Mod/Użytkownik czy ktokolwiek.


Zrobiłem osobne klasy do sprawdzenia poziomu (Anonim, User, Admin)
  1. class CheckMode
  2. {
  3. static public function isAnonymous(User $user)
  4. {
  5. return ($user -> getMode() == MODE_GUEST ? true : false);
  6. }
  7.  
  8. static public function isAdmin(User $user)
  9. {
  10. return ($user -> getMode() == MODE_ADMIN ? true : false);
  11. }
  12.  
  13. static public function isUser(User $user)
  14. {
  15. return ($user -> getMode() == MODE_USER ? true : false);
  16. }
  17. }


Zrobiłem też osobne klasy do rejestracji oraz sprawdzania Usera biggrin.gif

  1. class CheckUser
  2. {
  3. static public function run(httpRequest $req, DB $pdo)
  4. {
  5. $stmt = $pdo -> prepare('SELECT id, email, name, password, description, avatar, mode FROM '.MySql_TPrefix.'users WHERE email=:email AND password=:password');
  6. $stmt -> bindValue(':email', $req->getPost('email'), PDO::PARAM_STR);
  7. $stmt -> bindValue(':password', sha1(md5('@TuJestBardzo_').md5($req->getPost('haslo')).md5('_TrudnaSol@')), PDO::PARAM_STR);
  8. $stmt -> execute();
  9. $stmt -> setFetchMode(PDO::FETCH_CLASS, 'User', array(0 => $pdo, 1=>false));
  10. if($user = $stmt -> fetch())
  11. {
  12. $stmt -> closeCursor();
  13. return $user;
  14. }
  15. else
  16. {
  17. $stmt -> closeCursor();
  18. return null;
  19. }
  20. }
  21. }
  22.  
  23. class AddUser
  24. {
  25. static public function run(httpRequest $req, DB $pdo)
  26. {
  27. $result = User::checkUser($req,$pdo);
  28. if($result instanceof user){ return false; }
  29. $stmt = $pdo -> prepare('INSERT INTO '.MySql_TPrefix.'users (email, password, mode, name) VALUES (:email, :password, :mode, :name)');
  30. $stmt -> bindValue(':email', $req->getPost('email'), PDO::PARAM_STR);
  31. $stmt -> bindValue(':password', sha1(md5('@TuJestBardzo_').md5($req->getPost('haslo')).md5('_TrudnaSol@')), PDO::PARAM_STR);
  32. $stmt -> bindValue(':mode', MODE_USER, PDO::PARAM_INT);
  33. $stmt -> bindValue(':name', $req->getPost('nazwa'), PDO::PARAM_STR);
  34. $ile = $stmt -> execute();
  35. if($ile > 0)
  36. {
  37. return true;
  38. }
  39. else
  40. {
  41. return false;
  42. }
  43. }
  44. }


Pozdrawiam
Rav
Dipter
Cytat
Hmm...
Kod
Fatal error: Non-static method PDO::__construct() cannot be called statically in C:\wamp\www\class\db.class.php on line 33

nie wiem co poszło nie tak...


Błąd jest chyba logicznie prosty (Niestety mój tongue.gif) - Metoda connect() nie może być w tym przypadku statyczna ->

  1.  
  2.  
  3. DB::getInstance()->connect([...]);
  4. DB::getInstance()->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
  5.  


Klasa CheckUser i AddUser jest na razie bezsensu napisana, dlaczego? Przypatrz się uważnie - Skoro robisz jedną metodę w klasie to oznacza to, że klasa to tylko opakowanie na funkcję tongue.gif

Natomiast co do klasy CheckMode, ja nie mam żadnych zastrzeżeń (przynajmniej jakichś większych).

BTW:
Dlaczego w klasie DB nie przypiszesz takich stałych jak prefix?

pozdrawiam.
rav1989
Cytat
Błąd jest chyba logicznie prosty (Niestety mój tongue.gif) - Metoda connect() nie może być w tym przypadku statyczna ->

  1.  
  2.  
  3. DB::getInstance()->connect([...]);
  4. DB::getInstance()->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
  5.  


Błąd jest nadal czepia się tej linii:
  1. parent::__construct($config['dsn'], $config['username'], $config['password'], $config['driver_options']);

prawdopodobnie chodzi mu o ten zapis parent::__construct(...); bo to wskazuje, że konstruktor rodzica jest statyczny...

Cytat
Klasa CheckUser i AddUser jest na razie bezsensu napisana, dlaczego? Przypatrz się uważnie - Skoro robisz jedną metodę w klasie to oznacza to, że klasa to tylko opakowanie na funkcję tongue.gif

Hmm faktycznie, a teraz:
  1. class ValidateUser
  2. {
  3. static public function isExist(httpRequest $req, DB $pdo)
  4. {
  5. $stmt = $pdo -> prepare('SELECT id, email, name, password, description, avatar, mode FROM '.MySql_TPrefix.'users WHERE email=:email AND password=:password');
  6. $stmt -> bindValue(':email', $req->getPost('email'), PDO::PARAM_STR);
  7. $stmt -> bindValue(':password', sha1(md5('@TuJestBardzo_').md5($req->getPost('haslo')).md5('_TrudnaSol@')), PDO::PARAM_STR);
  8. $stmt -> execute();
  9. $stmt -> setFetchMode(PDO::FETCH_CLASS, 'User', array(0 => $pdo, 1=>false));
  10. if($user = $stmt -> fetch())
  11. {
  12. $stmt -> closeCursor();
  13. return $user;
  14. }
  15. else
  16. {
  17. $stmt -> closeCursor();
  18. return null;
  19. }
  20. }
  21. static public function add(httpRequest $req, DB $pdo)
  22. {
  23. $result = self::isExist($req,$pdo);
  24. if($result instanceof user){ return false; }
  25. $stmt = $pdo -> prepare('INSERT INTO '.MySql_TPrefix.'users (email, password, mode, name) VALUES (:email, :password, :mode, :name)');
  26. $stmt -> bindValue(':email', $req->getPost('email'), PDO::PARAM_STR);
  27. $stmt -> bindValue(':password', sha1(md5('@TuJestBardzo_').md5($req->getPost('haslo')).md5('_TrudnaSol@')), PDO::PARAM_STR);
  28. $stmt -> bindValue(':mode', MODE_USER, PDO::PARAM_INT);
  29. $stmt -> bindValue(':name', $req->getPost('nazwa'), PDO::PARAM_STR);
  30. $ile = $stmt -> execute();
  31. if($ile > 0)
  32. {
  33. return true;
  34. }
  35. else
  36. {
  37. return false;
  38. }
  39. }
  40. }

Cytat
BTW:
Dlaczego w klasie DB nie przypiszesz takich stałych jak prefix?

No wiesz, jak będę przenosił stronę na inny serwer to prefix się może zmienić i wtedy będę musiał szukać go po klasie a tak mam w jednym miejscu na widoku razem z innymi danymi biggrin.gif (Jest mniejsze prawdopodobieństwo, że o nim zapomnę).

Pozdrawiam
Rav
Dipter
Usunąłeś przed metodą w klasie DB magiczne słowo "static"? wink.gif
rav1989
Cytat(Dipter @ 27.04.2011, 19:59:39 ) *
Usunąłeś przed metodą w klasie DB magiczne słowo "static"? wink.gif


Nic nie usuwałem... słowo...
  1. class DB extends PDO {
  2.  
  3. static private $instance = null;
  4.  
  5. public function __construct() {}
  6.  
  7. static public function getInstance() {
  8. if(self::$instance === null) {
  9. self::$instance = new self();
  10. }
  11. return self::$instance;
  12. }
  13.  
  14. static public function connect($config = array()) {
  15. parent::__construct($config['dsn'], $config['username'], $config['password'], $config['driver_options']);
  16. }
  17. }

Jak widzisz skopiowałem twoją wersję biggrin.gif dodałem tylko $config['driver_options']

Rozumiem, że reszta jest już OK?

Pozdrawiam
Rav
Dipter
Właśnie o to chodzi by usunąć tego statica, bo on jest tutaj problemem. Pamiętaj też, że podałem przykład i nie należy go kopiować jak leci tongue.gif
rav1989
Cytat(Dipter @ 27.04.2011, 22:47:34 ) *
Właśnie o to chodzi by usunąć tego statica, bo on jest tutaj problemem. Pamiętaj też, że podałem przykład i nie należy go kopiować jak leci tongue.gif

Aaa to dobre! test na czujność biggrin.gif a ja jak głupi dałem się nabrać... ech...
Oczywiście teraz wszystko działa jak trzeba smile.gif

Mam nadzieję, że reszta jest w porządku biggrin.gif (Mam wątpliwość co do klasy httpRequest rozmyślam czy aby na pewno jest mi ona potrzebna)

Pozdrawiam
Rav
Dipter
Przydać zawsze się przyda, lecz najpierw trzeba wyznaczyć czym dana klasa ma się zajmować / co zwracać i trzeba przemyśleć jak ją napisać. Pamiętaj, że nie piszesz po to by było ładnie, ale po to by było to wygodne i pomocne. Jeśli wiesz, że nie jest Ci potrzebna, a tylko masz przez nią problemy - Zostaw ją w spokoju, usuń wink.gif
rav1989
Cytat(Dipter @ 27.04.2011, 23:09:14 ) *
Przydać zawsze się przyda, lecz najpierw trzeba wyznaczyć czym dana klasa ma się zajmować / co zwracać i trzeba przemyśleć jak ją napisać. Pamiętaj, że nie piszesz po to by było ładnie, ale po to by było to wygodne i pomocne. Jeśli wiesz, że nie jest Ci potrzebna, a tylko masz przez nią problemy - Zostaw ją w spokoju, usuń wink.gif


Pomyślałem i wymyśliłem takie coś:

  1. <?php
  2.  
  3. class SendHttpRequest
  4. {
  5. static public function cookie($name,$value,$expire=3600)
  6. {
  7. setcookie($name, $value,time()+$expire);
  8. }
  9.  
  10. static public function session($key,$value)
  11. {
  12. $_SESSION[$key] = $value;
  13. }
  14.  
  15. static public function get($key,$value)
  16. {
  17. $_SESSION[$key] = $value;
  18. }
  19.  
  20. static public function post($key,$value)
  21. {
  22. $_SESSION[$key] = $value;
  23. }
  24. }
  25.  
  26. class GetHttpRequest
  27. {
  28. static public function cookie($key = null)
  29. {
  30. if(($key == null ? (!empty($_COOKIE)) : (isset($_COOKIE[$key]))))
  31. {
  32. return ($key == null ? $_COOKIE : $_COOKIE[$key]);
  33. }else{
  34. return null;
  35. }
  36. }
  37.  
  38. static public function session($key = null)
  39. {
  40. if(($key == null ? (!empty($_SESSION)) : (isset($_SESSION[$key]))))
  41. {
  42. return ($key == null ? $_SESSION : $_SESSION[$key]);
  43. }else{
  44. return null;
  45. }
  46. }
  47.  
  48.  
  49. static public function get($key = null)
  50. {
  51. return ($key == null ? $_GET : $_GET[$key]);
  52. }
  53.  
  54. static public function post($key = null)
  55. {
  56. return ($key == null ? $_POST : $_POST[$key]);
  57. }
  58.  
  59. static public function server($key = null)
  60. {
  61. return ($key == null ? $_SERVER : $_SERVER[$key]);
  62.  
  63. }
  64.  
  65. static public function ip()
  66. {
  67. return $_SERVER['REMOTE_ADDR'];
  68. }
  69.  
  70. static public function browser()
  71. {
  72. return $_SERVER['HTTP_USER_AGENT'];
  73. }
  74. }
  75.  
  76. class ValidateInput
  77. {
  78. static public function isEmail($input)
  79. {
  80. return (!preg_match("/^([a-z0-9+_-]+)(.[a-z0-9+_-]+)*@([a-z0-9-]+.)+[a-z]{2,6}$/ix", $input)) ? false : true;
  81. }
  82.  
  83. static public function isUrl($input)
  84. {
  85. return (!preg_match("|https?://([-\w\.]+)+(:\d+)?(/([\w/_\.]*(\?\S+)?)?)?|ix", $input)) ? false : true;
  86. }
  87.  
  88. static public function isValidPassword($input)
  89. {
  90. return (!preg_match("/^[[:ascii:][:alnum:]]{5,}$/ix", $input)) ? false : true;
  91. }
  92. }
  93.  
  94. ?>


Przydałoby się też sprawdzić w przypadku emailu oraz adresu url czy istnieją naprawdę a nie tylko czy jest poprawna składnia... tyle, że nie mam pojęcia jak to zrobić... oraz nie wiem czy się opłaca (wydłużyłoby to proces rejestracji).

Pozdrawiam
Rav
To jest wersja lo-fi głównej zawartości. Aby zobaczyć pełną wersję z większą zawartością, obrazkami i formatowaniem proszę kliknij tutaj.
Invision Power Board © 2001-2025 Invision Power Services, Inc.