user class + dziedziczenie, Ocena klasy Opcje

[PawelC]

Hej piszę sobie klasę users, która jest odpowiedzialna za logowanie i dodanie użytkownika, później dojdzie kasowanie i edycja konta. Moje pytanie brzmi czy sposób w który piszę jest dobry:

[PHP] pobierz, plaintext 
<?
class user extends sqli
  {
 
    public function userLogin()
      {    
 
       $this->log=trim($_POST['log']);
       $this->pass=$pass=trim($_POST['pass']);
       $result=$this->mysqli->query("select log,pass from users where log='$this->log' and pass='$this->pass'");
        if(mysqli_num_rows($result)==0)
            {
                echo "brak takiego usera";
             }
             else
             {
                $_SESSION['zalogowany']="$this->log";
                header("Location: test.php");
              }
 
 
            }
            
            
   public function addUser()
            {
 
             $this->log=trim($_POST['log']);
             $this->pass=$pass=trim($_POST['pass']);
             $result=$this->mysqli->query("insert into users values('$this->log','$this->pass')");
             if($result===true)
              {
                  echo "Użytkownik został dodany poprawnie.";
              }
               else
               {
                  echo "Wystąpił błąd podczas dodawania nowego użytkownika!";
              }
 
            }
 
 
 
        }
$user=new user('localhost','root','','katalog');
?>
[PHP] pobierz, plaintext 

I dodawanie użytkownika i samo logowanie działa poprawnie, a mi chodzi o Waszą ocenę czy to jest dobrze napisane. Oczywiście zdaję sobie sprawę, że można to napisać zawsze lepiej.

Ten post edytował ExPlOiT 2.05.2009, 22:57:27

[ayeo]

Witam!

Nie jest różowo (IMG:http://forum.php.pl/style_emoticons/default/smile.gif) Na początek taki detal, nazwy klas dobrze zaczynać wielką literą. Twoja klasa dziedziczy po sqli (?). Podejrzewam, że to coś co odpowiada za połączenie z bazą itd. Czyli każda instancja tej klasy tworzy nowe połączenie? Klasę do obsługi bazy danych zrób sobie jako Singleton (najprościej). Nie dziedzicz po niej. Metoda loginUser() powinna przyjmować parametry, a nie czytać z posta. Może od początku. Obiekt user reprezentuje użytkownika, mianowicie jego id, imię, nazwisko, login, email i hasło (dla przykładu). Fajnie zrobić statyczną metodę login( $login, $password ), która po prostu zwróci obiekt User wypełniny kompletem danych z bazy lub false w przypadku błędnych danych logowania.

Dodawanie Usera... Moim zdaniem (podkreślam, że to moje zdanie) powinno to wyglądać tak:

[PHP] pobierz, plaintext 
<?php
$user = new User;
$user->login= "adam";
$user->password = "dupa8";
$user->save();
?>
[PHP] pobierz, plaintext 

Oczywiście można dorobić jakiś obiekt bazowy, który ma metody typu setData( $post ) - czyli ustawia właściwości obiektu na podstawie danych z tablicy $_POST itd. Jednak nie o to chodzi. Obiekt to nie pojemnik na funkcje. Niech faktycznie będzie to obiekt User.

Statyczna metoda login(); powinna ustawiac też flagę w obiekcie czy dane są z bazy czy jest to nowy obiekt User. Wtedy metoda save() wie czy robić insert czy update. Przdałyby się jakieś walidatory jako prywatne metody (w uproszczeniu). Przykład: tworzę nowego Usera, podaję login (ale nie podaję hasła). Robię $user->save(); metoda powinna wyrzucić stosowny wyjątek.

Tak bym to widział.

Pozdrawiam!