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ć (IMG:style_emoticons/default/biggrin.gif)

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 (IMG:style_emoticons/default/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 (IMG:style_emoticons/default/biggrin.gif) natomiast ja czekam na dalsze propozycję optymalizacji...

Pozdrawiam
Rav