Optymalizacja klasy do obsługi użytkownika Opcje

[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ć

Klasa:

[PHP] pobierz, plaintext 
session_start();
 
define("MySql_Host", "localhost");
define("MySql_Login", "root");
define("MySql_Password", "");
define("MySql_DbName", "test");
define("MySql_TPrefix", "");
 
define("MODE_USER",0);
define("MODE_MOD",1);
define("MODE_ADMIN",2);
 
function removeSlashes(&$value)
		{
             if(is_array($value))
            {
                return array_map('removeSlashes', $value);
            }
            else
            {
                 return stripslashes($value);
            }
		}
 
class User{
private $id;
private $email;
private $haslo;
 
public $nazwa;
public $opis;
public $avatar;
public $mode;
public $urls;
private $sesja;
 
private $pdo;
 
public function __construct($email_ = null, $haslo_ = null){
$this->pdo = new PDO('mysql:host='.MySql_Host.';dbname='.MySql_DbName, MySql_Login , MySql_Password);
$this->pdo -> setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
 
if(isset($email_) and isset($haslo_)){
$this->email = $email_;
$this->haslo = md5(sha1($haslo_).sha1('TuJestSol'));
}elseif(isset($_SESSION['token'])){
$this->sesja = $_SESSION['token'];
$this->Load();
}else die('Ups! Odwiedź rejestrację!');
}
 
private function mysql_insert($table, $inserts) {
    $values = array_map("removeSlashes", array_values($inserts));
    $keys = array_keys($inserts);
 
	$query = 'INSERT INTO `'.MySql_TPrefix.$table.'` (`'.implode('`,`', $keys).'`) VALUES (\''.implode('\',\'', $values).'\')';
	try{
    return $this->pdo -> exec($query);
	}
	catch(PDOException $e)
        {
                echo 'Wystąpił błąd biblioteki PDO: ' . $e->getMessage();
				return false;
        }
}
 
public function Rejestruj($nazwa_){
$this->nazwa = $nazwa_;
$this->mode = MODE_USER;
      $stmt = $this->pdo -> query('SELECT * FROM `users` WHERE `haslo`="'.$this->haslo.'" and `email`="'.$this->email.'"');
	  if($stmt->fetch()) return false;
$tmp = $this->mysql_insert('users',array('nazwa'=>$this->nazwa, 'haslo'=>$this->haslo, 'email'=>$this->email, 'mode'=>$this->mode));
 
	if($tmp)
		return true;
		return false;
}
 
public function Loguj(){
session_start();
   try
   {
		$_SESSION['token'] = md5(rand());
		$this->sesja = $_SESSION['token'];
      $this->pdo -> query('UPDATE `users` SET `sesja`="'.$_SESSION['token'].'" WHERE `haslo`="'.$this->haslo.'" and `email`="'.$this->email.'"');
   }
   catch(PDOException $e)
   {
      echo 'Połączenie nie mogło zostać utworzone: ' . $e->getMessage();
	  return null;
   }
}
public function Wyloguj(){
   try
   {
      $this->pdo -> query('UPDATE `users` SET `sesja`="NULL" WHERE `sesja`="'.$this->sesja.'"');
   }
   catch(PDOException $e)
   {
      echo 'Połączenie nie mogło zostać utworzone: ' . $e->getMessage();
	  return null;
   }
   session_destroy();
}
 
public function __set($name,$value){
$this->$name($value);
}
private function Load(){
   try
   {
      $stmt = $this->pdo -> query('SELECT * FROM `users` WHERE `sesja`="'.$this->sesja.'"');
	  $row = $stmt->fetch();
      $stmt -> closeCursor(); 
	  $urls = $this->pdo -> query('SELECT url FROM `user_sites` WHERE `users_id`="'.$row['id'].'"'); 
	  foreach($urls as $url){
	  $this->urls[] = base64_decode($url['url']);
	  }
	  $urls -> closeCursor();
	  $this->opis = $row['opis'];
	  $this->avatar = $row['avatar'];
	  $this->nazwa = $row['nazwa'];
	  $this->mode = $row['mode'];
	  $this->id = $row['id'];
   }
   catch(PDOException $e)
   {
      echo 'Połączenie nie mogło zostać utworzone: ' . $e->getMessage();
	  return null;
   }
}
 
public function AddUrl($url){
$url = base64_encode($url);
$stmt = $this->pdo -> query('SELECT * FROM `user_sites` WHERE `url`="'.$url.'" and `users_id`="'.$this->id.'"'); 
if($stmt->fetch()) return false;
$tmp = $this->mysql_insert('user_sites',array('url'=>$url, 'users_id'=>$this->id));
$this->urls[] = base64_decode($url);
 
	if($tmp)
		return true;
		return false;
}
 
public function Opis($value){
$this->opis = $value;
   try
   {
      $this->pdo -> query('UPDATE `users` SET `opis`="'.$this->opis.'" WHERE `sesja`="'.$this->sesja.'"');
   }
   catch(PDOException $e)
   {
      echo 'Połączenie nie mogło zostać utworzone: ' . $e->getMessage();
	  return null;
   }
}
 
public function Avatar($value){
$this->avatar = $value;
   try
   {
      $this->pdo -> query('UPDATE `users` SET `avatar`="'.$this->avatar.'" WHERE `sesja`="'.$this->sesja.'"');
   }
   catch(PDOException $e)
   {
      echo 'Połączenie nie mogło zostać utworzone: ' . $e->getMessage();
	  return null;
   }
}
 
public function Mode($value){
$this->mode = $value;
   try
   {
      $this->pdo -> query('UPDATE `users` SET `mode`="'.$this->mode.'" WHERE `sesja`="'.$this->sesja.'"');
   }
   catch(PDOException $e)
   {
      echo 'Połączenie nie mogło zostać utworzone: ' . $e->getMessage();
	  return null;
   }
}
 
 
 
}
[PHP] pobierz, plaintext 

Logowanie:

[PHP] pobierz, plaintext 
require_once('class/user.class.php');
$user = new User('test','test');
$user -> Loguj();
[PHP] pobierz, plaintext 

Update/Odczyt danych:

[PHP] pobierz, plaintext 
require_once('class/user.class.php');
$user = new User();
$user->Opis = 'adad';
$user->AddUrl ='http://wp.pl';
echo $user -> opis;
print_r($user -> urls);
[PHP] pobierz, plaintext 

Wylogowanie:

[PHP] pobierz, plaintext 
require_once('class/user.class.php');
$user = new User();
$user->Wyloguj();
[PHP] pobierz, plaintext 

Struktura SQL:

[SQL] pobierz, plaintext 
CREATE  TABLE IF NOT EXISTS `users` (
  `id` INT NOT NULL AUTO_INCREMENT ,
  `nazwa` VARCHAR(45) NULL ,
  `email` VARCHAR(45) NULL ,
  `haslo` VARCHAR(45) NULL ,
  `opis` TEXT NULL ,
  `avatar` VARCHAR(128) NULL ,
  `mode` INT NULL ,
  `sesja` VARCHAR(45) NULL ,
  PRIMARY KEY (`id`) )
ENGINE = MyISAM
DEFAULT CHARACTER SET = utf8
COLLATE = utf8_general_ci
PACK_KEYS = DEFAULT;
[SQL] pobierz, plaintext 

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]

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
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 natomiast ja czekam na dalsze propozycję optymalizacji...

Pozdrawiam
Rav

[misiek172]

można wiele zmienić taka troszkę średnia ta klasa

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]

można wiele zmienić taka troszkę średnia ta klasa

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 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

[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

[rav1989]

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

Dziękuję Ci za te wskazówki

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

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

Ten post edytował rav1989 25.04.2011, 21:47:50

[konole]

Postaraj się trzymać stałego nazewnictwa. Nazwy funkcji zaczynaj od małej litery, nie wielkiej (klasa User: Rejestruj itd.)

[rav1989]

Postaraj się trzymać stałego nazewnictwa. Nazwy funkcji zaczynaj od małej litery, nie wielkiej (klasa User: Rejestruj itd.)

OK, już to poprawiłem

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

Pozdrawiam
Rav

Ten post edytował rav1989 25.04.2011, 22:26:58

[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
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 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

[rav1989]

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

Ok Dodałem przechowywanie POST oraz GET

Tak to teraz wygląda

[PHP] pobierz, plaintext 
class httpRequest
{
	private $ip;
	private $browser;
	private $post;
	private $get;
 
	public function __set($name,$value)
	{
		$this -> $name = $value;
	}
 
	public function sendGet($array)
	{
		$_GET = $array;
	}
 
	public function sendPost($array)
	{
		$_POST = $array;
	}
 
	public function getGet($key = null)
	{
		return ($key == null ? $this->get : $this->get[$key]);
	}
 
	public function getPost($key = null)
	{
		return ($key == null ? $this->post : $this->post[$key]);
	}
 
	public function getIp()
	{
		return $this -> ip;
	}
 
	public function getBrowser()
	{
		return $this -> browser;
	}
}
[PHP] pobierz, plaintext 

A tak się inicjalizuje

[PHP] pobierz, plaintext 
$request = new httpRequest;
 
$request -> ip = $_SERVER['REMOTE_ADDR'];
$request -> browser = $_SERVER['HTTP_USER_AGENT'];
$request -> get = $_GET;
$request -> post = $_POST;
[PHP] pobierz, plaintext 

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:

[PHP] pobierz, plaintext 
class DB extends PDO
{
	private static $instance;
	private function __clone(){}
 
	public static function getInstance ($dsn, $username = null, $password = null, $driver_options = null)
	{
		if (self::$instance === null) {
			self::$instance = new self($dsn, $username, $password, $driver_options);
			self::$instance  -> setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
		}
		return self::$instance;
	}
}
[PHP] pobierz, plaintext 

[PHP] pobierz, plaintext 
$pdo =  DB::getInstance('mysql:host='.MySql_Host.';dbname='.MySql_DbName, MySql_Login , MySql_Password);
[PHP] pobierz, plaintext 

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

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

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 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)

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]

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 getInstance() powinien zwracać Ci obiekt klasy DB, oraz przy okazji klasy PDO.

Przykład:

[PHP] pobierz, plaintext 
 
class DB extends PDO {
 
	static private $instance = null;
 
	public function __construct() {}
 
	static public function getInstance() {
		if(self::$instance === null) {
			self::$instance = new self();
		}
		return self::$instance;
	}
 
	static public function connect($config = array()) {
		parent::__construct($config['dsn'], $config['username'], $config['password'], [...]);
	}
 
}
 
[PHP] pobierz, plaintext 

I wykorzystanie na początku aplikacji?

[PHP] pobierz, plaintext 
 
DB::connect(array([...]));
 
DB::getInstance()->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
 
[PHP] pobierz, plaintext 

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

[rav1989]

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

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

Przykład:

[PHP] pobierz, plaintext 
 
class DB extends PDO {
 
	static private $instance = null;
 
	public function __construct() {}
 
	static public function getInstance() {
		if(self::$instance === null) {
			self::$instance = new self();
		}
		return self::$instance;
	}
 
	static public function connect($config = array()) {
		parent::__construct($config['dsn'], $config['username'], $config['password'], [...]);
	}
 
}
 
[PHP] pobierz, plaintext 

I wykorzystanie na początku aplikacji?

[PHP] pobierz, plaintext 
 
DB::connect(array([...]));
 
DB::getInstance()->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
 
[PHP] pobierz, plaintext 

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...

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)

[PHP] pobierz, plaintext 
class CheckMode
{
	static public function isAnonymous(User $user)
	{
		return ($user -> getMode() == MODE_GUEST ? true : false);
	}
 
	static public function isAdmin(User $user)
	{
		return ($user -> getMode() == MODE_ADMIN ? true : false);
	}
 
	static public function isUser(User $user)
	{
		return ($user -> getMode() == MODE_USER ? true : false);
	}
}
[PHP] pobierz, plaintext 

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

[PHP] pobierz, plaintext 
class CheckUser
{
	static public function run(httpRequest $req, DB $pdo)
	{
		$stmt = $pdo -> prepare('SELECT id, email, name, password, description, avatar, mode FROM '.MySql_TPrefix.'users WHERE email=:email AND password=:password');
		$stmt -> bindValue(':email', $req->getPost('email'), PDO::PARAM_STR);
		$stmt -> bindValue(':password', sha1(md5('@TuJestBardzo_').md5($req->getPost('haslo')).md5('_TrudnaSol@')), PDO::PARAM_STR);
		$stmt -> execute();
		$stmt -> setFetchMode(PDO::FETCH_CLASS, 'User', array(0 => $pdo, 1=>false));
		if($user = $stmt -> fetch())
		{
			$stmt -> closeCursor();
			return $user;
		}
		else
		{
			$stmt -> closeCursor();
			return null;
		}
	}
}
 
class AddUser
{
	static public function run(httpRequest $req, DB $pdo)
	{
		$result = User::checkUser($req,$pdo);
		if($result  instanceof user){ return false; }
		$stmt = $pdo -> prepare('INSERT INTO '.MySql_TPrefix.'users (email, password, mode, name) VALUES (:email, :password, :mode, :name)');
		$stmt -> bindValue(':email', $req->getPost('email'), PDO::PARAM_STR);
		$stmt -> bindValue(':password', sha1(md5('@TuJestBardzo_').md5($req->getPost('haslo')).md5('_TrudnaSol@')), PDO::PARAM_STR);
		$stmt -> bindValue(':mode', MODE_USER, PDO::PARAM_INT);
		$stmt -> bindValue(':name', $req->getPost('nazwa'), PDO::PARAM_STR);
		$ile = $stmt -> execute();
		if($ile > 0)
		{
			return true;
		}
		else
		{
			return false;
		}
	}
}
[PHP] pobierz, plaintext 

Pozdrawiam
Rav

[Dipter]

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 ) - Metoda connect() nie może być w tym przypadku statyczna ->

[PHP] pobierz, plaintext 
 
 
DB::getInstance()->connect([...]);
DB::getInstance()->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
 
[PHP] pobierz, plaintext 

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ę

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]

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

[PHP] pobierz, plaintext 
 
 
DB::getInstance()->connect([...]);
DB::getInstance()->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
 
[PHP] pobierz, plaintext 

Błąd jest nadal czepia się tej linii:

[PHP] pobierz, plaintext 
parent::__construct($config['dsn'], $config['username'], $config['password'], $config['driver_options']);
[PHP] pobierz, plaintext 

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

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ę

Hmm faktycznie, a teraz:

[PHP] pobierz, plaintext 
class ValidateUser
{
	static public function isExist(httpRequest $req, DB $pdo)
	{
		$stmt = $pdo -> prepare('SELECT id, email, name, password, description, avatar, mode FROM '.MySql_TPrefix.'users WHERE email=:email AND password=:password');
		$stmt -> bindValue(':email', $req->getPost('email'), PDO::PARAM_STR);
		$stmt -> bindValue(':password', sha1(md5('@TuJestBardzo_').md5($req->getPost('haslo')).md5('_TrudnaSol@')), PDO::PARAM_STR);
		$stmt -> execute();
		$stmt -> setFetchMode(PDO::FETCH_CLASS, 'User', array(0 => $pdo, 1=>false));
		if($user = $stmt -> fetch())
		{
			$stmt -> closeCursor();
			return $user;
		}
		else
		{
			$stmt -> closeCursor();
			return null;
		}
	}
	static public function add(httpRequest $req, DB $pdo)
	{
		$result = self::isExist($req,$pdo);
		if($result  instanceof user){ return false; }
		$stmt = $pdo -> prepare('INSERT INTO '.MySql_TPrefix.'users (email, password, mode, name) VALUES (:email, :password, :mode, :name)');
		$stmt -> bindValue(':email', $req->getPost('email'), PDO::PARAM_STR);
		$stmt -> bindValue(':password', sha1(md5('@TuJestBardzo_').md5($req->getPost('haslo')).md5('_TrudnaSol@')), PDO::PARAM_STR);
		$stmt -> bindValue(':mode', MODE_USER, PDO::PARAM_INT);
		$stmt -> bindValue(':name', $req->getPost('nazwa'), PDO::PARAM_STR);
		$ile = $stmt -> execute();
		if($ile > 0)
		{
			return true;
		}
		else
		{
			return false;
		}
	}
}
[PHP] pobierz, plaintext 

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 (Jest mniejsze prawdopodobieństwo, że o nim zapomnę).

Pozdrawiam
Rav

[Dipter]

Usunąłeś przed metodą w klasie DB magiczne słowo "static"?

[rav1989]

Usunąłeś przed metodą w klasie DB magiczne słowo "static"?

Nic nie usuwałem... słowo...

[PHP] pobierz, plaintext 
class DB extends PDO {
 
	static private $instance = null;
 
	public function __construct() {}
 
	static public function getInstance() {
		if(self::$instance === null) {
			self::$instance = new self();
		}
		return self::$instance;
	}
 
	static public function connect($config = array()) {
		parent::__construct($config['dsn'], $config['username'], $config['password'], $config['driver_options']);
	}
} 
[PHP] pobierz, plaintext 

Jak widzisz skopiowałem twoją wersję 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

[rav1989]

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

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

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

Pozdrawiam
Rav

Ten post edytował rav1989 27.04.2011, 22:06:23

[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ń