Oceńcie czy kod jest ok Opcje

[marekc12]

[PHP] pobierz, plaintext 
<?php
class bazaDanych
    {
        private $serwer, $nazwa, $uzytkownik, $haslo, $idPolaczenia, $wynikZapytania;
        
        function __construct( $serwer, $nazwa, $uzytkownik, $haslo )
        {
            $this->serwer = $serwer;
            $this->nazwa = $nazwa;
            $this->uzytkownik = $uzytkownik;
            $this->haslo = $haslo;
            
            $this->idPolaczenia = mysql_connect( $this->serwer, $this->uzytkownik, $this->haslo );
            if( !$this->idPolaczenia )
                $this->blad();
 
            $dbSelected = mysql_select_db( $this->nazwa, $this->idPolaczenia );
            if( !$dbSelected )
                $this->blad();
        }
 
        function __destruct() {
            mysql_close( $this->idPolaczenia );
        }
        
        function modyfikujBaze( $trescZapytania )
        {
            $this->wynikZapytania = mysql_query( $trescZapytania, $this->idPolaczenia )
            or $this->blad( $trescZapytania );
        }
        
        function czytajBaze( $trescZapytania )
        {
            $this->modyfikujBaze( $trescZapytania );
            
            while ($row = mysql_fetch_array( $this->wynikZapytania ))
                $table[] = $row;
            
            return $table;
        }
 
        private function blad( $trescZapytania ) {
            die( mysql_errno( $this->idPolaczenia ) . ': ' . mysql_error( $this->idPolaczenia ) );
        }
    }
 
    class uzytkownik
    {
        private $bazaDanych, $id, $uprawnienia, $login, $haslo, $imie;
        
        function __construct( $bazaDanych, $id, $uprawnienia, $login, $haslo, $imie )
        {
            $this->bazaDanych = $bazaDanych;
            $this->id = $id;
            $this->uprawnienia = $uprawnienia;
            $this->login = $login;
            $this->haslo = $haslo;
            $this->imie = $imie;
        }
        
        function dodaj() {
            $this->bazaDanych -> modyfikujBaze( "INSERT INTO users (id, rights, login, pass, name) VALUES ('', '$this->uprawnienia', '$this->login', '$this->haslo', '$this->imie')\" );
        }
        
        function usun() {
            $this->bazaDanych -> modyfikujBaze( "DELETE FROM users WHERE id=$this->id&#092;" );
        }
 
        function edytuj() {
            $this->bazaDanych -> modyfikujBaze( "UPDATE users SET rights='$this->uprawnienia', login='$this->login', pass='$this->haslo', login='$name->imie' WHERE id=$this->id\" );
        }
        
        function wczytajPozycje() {
            return $this->bazaDanych -> czytajBaze("SELECT * FROM users WHERE id=$this->id&#092;" );
        }
    }
?>
[PHP] pobierz, plaintext 

[PHP] pobierz, plaintext 
<?php
$mySql = new bazaDanych( 'localhost', 'lala', 'asdas', 'asdkl' );
$osoba = new uzytkownik( $mySql, null, 1, 'login123', 'mojehaslo', 'marek' );
$osoba -> dodaj();
?>
[PHP] pobierz, plaintext 

Czy ten kod jest dobry? Uczę się obiektowości i zastanawiam się czy dobrze to napisałem. Baze danych zrobiłem jedną klasą, użytkownika drugą, to chyba jest dobrze. Tylko nie wiem czy dobrze to rozwiązałem, że do klasy użytkownik przekazuję obiekt bazy danych. Proszę, napiszcie mi co myślicie o tym kodzie, czy mogę pisać dalej, czy może to jest w ogóle bez sensu (IMG:http://forum.php.pl/style_emoticons/default/smile.gif) . Nie chodzi mi o idealny sposób na zapis tego kodu(każdy z Was pisze troszeczkę inaczej), tylko o to czy mogę to rozwiązać tak jak to zrobiłem, czy mój sposób nie kłóci się z zasadami programowania obektowego.

pozdrawiam!

Ten post edytował erix 12.06.2009, 14:13:14

Powód edycji: [erix] przeniosłem

[Spawnm]

zły dział...

klasa bazaDanych jest bez sensu , na siłe dałeś mysqla w klase i koniec ...
napisz singletona czy coś... bo obecna klasa to zwykły mysql tylko wolniejszy i ma zmienione nazwy.

uzytkownik

zbyt wiele wymagasz danych do constructa , klasa zbyt wymusza dane i wiele nie daje ...

[Fifi209]

Kod

$osoba = new uzytkownik( $mySql, null, 1, 'login123', 'mojehaslo', 'marek' );

A nie lepiej np.:

Kod

$osoba->setDabase($handle);
$osoba->login = 'login';
$osoba->password = 'haslo';
$osoba->save();

[marekc12]

uzylem tego singletona, w tej chwili to czy mamy podane $this->id jest równe temu czy użytkownik jest już zapisany w bazie. Myślicie że taka klasa jest dobra?

[PHP] pobierz, plaintext 
<?php
class Uzytkownik
{
//singleton
    private static $oInstance = false;
   private function __construct() {}
 
    public static function getInstance()
    {
        if( self::$oInstance == false )
            self::$oInstance = new Singleton();
        return self::$oInstance;
    }
//koniec: singleton
 
    public $imie, $nazwisko;
    private $id = null, $login = null, $haslo = null;
    
    function wybierzPozycje( $id )
    {
        $id = filtrMyql( $id );
        
        // jesli istnieje uzytkownik o takim ID wczytaj go z bazy        
        if( $wynik = czytajBaze( &#092;"SELECT * FROM uzytkownicy WHERE id=$id\" ) )
        {
            $this->id = $id;
            
            $this->login -> $wynik[1][2];
            $this->haslo -> $wynik[1][3];
            $this->imie -> $wynik[1][4];
            $this->nazwisko -> $wynik[1][5];
            
            return $wynik;
        }
        else
            return false;
    }
    
    function ustawLogin( $login )
    {
        //jesli uzytkownika nie ma jeszcze w bazie( nie mamy ustawionego ID) to zmieniamy login
        if ( $id )
        {
            $this->login = filtrMyql( $login );
            return true;
        }
        else
            return false;
    }
    
    function ustawHaslo( $haslo )
    {
        //szyfrujemy haslo    
        $this->haslo = md5( filtrMyql( $haslo ) );
    }
    
    function dodaj()
    {
        $imie = filtrMyql( $imie );
        $nazwisko = filtrMyql( $nazwisko );
        
        //tu jeszcze trzeba dopisać sprawdzanie czy login nie jest zajęty
            
        if( ! modyfikujBaze( &#092;"INSERT INTO uzytkownicy (id, login, haslo, imie, nazwisko) VALUES ('', '$this->login', '$this->haslo', '$this->imie', '$this->nazwisko')\" ) )
            return false;
    }
    
    function usun()
    {
        if( ! modyfikujBaze( &#092;"DELETE FROM uzytkownicy WHERE id=$this->id\" ) )
            return false;
    }
    
    function edytuj()
    {
        $imie = filtrMyql( $imie );
        $nazwisko = filtrMyql( $nazwisko );
        
        if( ! modyfikujBaze( &#092;"UPDATE uzytkownicy SET haslo='$this->haslo', imie='$name->imie', imie='$nazwisko->nazwisko' WHERE id=$this->id\" ) )
            return false;
    }
    
    function wczytajWszystkich()
    {
        $wynik = czytajBaze( &#092;"SELECT * FROM uzytkownicy\" ) )
        return $wynik;
    }
}
?>
[PHP] pobierz, plaintext 

Ten post edytował marekc12 25.06.2009, 13:24:40

[Crozin]

1) To nie jest poprawna implementacja singletona. Nadal istnieje możliwość stworzenia wielu obiektów - poprzez klonowanie.
2) Taka drobna rada. Dla Uzytkownik::oInstance ustaw domyślnie null, nie false. Widzę, że użyłeś tego śmiesznego sposobu zapisywania nazw zmiennych (gdzie pierwszy znak określa typ zmiennej - o: object) (właśnie, jak to się nazywało? :]) więc wypada być konsekwentnym i nie przypisywać do niej false (typ: boolean)

[em1X]

1) Konstruktor służy (w teorii) wyłącznie do przypisywania obiektowi pewnych wartości więc ten mysql_connect tam jest bez sensu, powinien się raczej znaleźć w metodzie connect(). W przypadku rozszerzania klasy miałbyś problem, poza tym ograniczasz sobie/innym pole manewru.

2) Pola w klasie mysql powinny być protected nie private. W przypadku rozszerzania klasy nie byłyby dostępne.

3) te przekazywanie uchwytu do mysqla dla użytkownika jest bez sensu, lepiej skorzystać z singletona bądź wzorca factory z jakiejś globalnej klasy

4) klasy nie powinny wyświetlać żadnych błędów, ponieważ narzucasz je z góry mimo, że mogą być niepotrzebne. Zamiast tego wyrzucaj wyjątek, który ktoś może obsłużyć lub nie - ma wybór.

Ten post edytował em1X 25.06.2009, 15:51:42

[hostingekspert]

Kod

//singleton
   private static $oInstance = false;
  private function __construct() {}

   public static function getInstance()
   {
       if( self::$oInstance == false )
           self::$oInstance = new Singleton();
       return self::$oInstance;
   }
//koniec: singleton

1) prywatny konstruktor??
2) self::$oInstance = new Singleton(); (IMG:http://forum.php.pl/style_emoticons/default/questionmark.gif)

Kod

       if( $wynik = czytajBaze( \"SELECT * FROM uzytkownicy WHERE id=$id\" ) )
       {
           $this->id = $id;
          
           $this->login -> $wynik[1][2];
           $this->haslo -> $wynik[1][3];
           $this->imie -> $wynik[1][4];
           $this->nazwisko -> $wynik[1][5];
          
           return $wynik;
       }

3) gdzie masz definicję metody czytajBaze i dlaczego tablica dwuwymiarowa??

chyba ciut za bardzo kombinujesz, zapoznaj się z dokumentacją PDO, SQLite a najlepiej MySQLi

[em1X]

ja bym dał

[PHP] pobierz, plaintext 
<?php
protected static $instance=null;
protected function __construct() {}
 
public static function getInstance($class=__CLASS__)
{
   return empty(self::$instance) ? self::$instance=new $class : self::$instance;
}
?>
[PHP] pobierz, plaintext 

ponieważ można kopiować prędko do każdej klasy bez potrzeby pisania zmian (IMG:http://forum.php.pl/style_emoticons/default/smile.gif)

[marekc12]

a akurat singletona to skopiowałem z innej stronki, chodziło mi bardziej o reszte kodu (i czy dobrze, że użyłem w tej klasie singletona? bo nie trzeba mi wiecej niz 1 obiektów tej klasy), czytaj baze to jest coś takiego(na szybko to pisalem moga byc jakies bledy, nie wazne):

[PHP] pobierz, plaintext 
<?php
function modyfikujBaze( $trescZapytania )
       {
           $wynikZapytania = mysql_query( $trescZapytania, $this->idPolaczenia )
 return $wynikZapytania;          
       }
       
       function czytajBaze( $trescZapytania )
       {
 $wynikZapytania = modyfikujBaze( $trescZapytania );
           
           while ($row = mysql_fetch_array( $wynikZapytania ))
               $table[] = $row;
           
           return $table;
       }
?>
[PHP] pobierz, plaintext 

[em1X]

te nazwy to z kosmosu chyba bierzesz (IMG:http://forum.php.pl/style_emoticons/default/smile.gif)

[PHP] pobierz, plaintext 
<?php
function execute($sql)
{
   $rs=@mysql_query($sql, $this->idPolaczenia);
   if (mysql_errno($this->idPolaczenia)) {
      throw new Exception($sql . ': ' . mysql_error(), mysql_errno());
   }
 
   return $rs;
}
 
function & query($sql)
{
   $rs=$this->execute($sql);
   $tab=$row=array();
 
   while($row=mysql_fetch_array($rs)) {
      $tab[]=$row;
   }
 
   @mysql_free_result($rs);
   return $tab;
}
?>
[PHP] pobierz, plaintext 

Ten post edytował em1X 26.06.2009, 09:20:39

[marekc12]

ok, dzięki za rady (IMG:http://forum.php.pl/style_emoticons/default/winksmiley.jpg) Co do nazewnictwa to wiem, że może jest trochę śmieszne, ale myślę, że lepiej jest się odnaleźć w kodzie, jeżli używa wymyślonych przez samego siebie nazw.

Poprawiłem(jeszcze mialem wrzucic wyjątki o których pisał em1X) klasę do bazy i obsługi użytkowników, zapewne nie jest idealna, ale wrzucam ją tu, może akurat komuś się przyda (IMG:http://forum.php.pl/style_emoticons/default/smile.gif) ) Pozdrawiam!

[PHP] pobierz, plaintext 
<?php
class bazaDanych
{
 //singleton
 protected static $instance=null;
 protected function __construct() {}
 public static function getInstance($class=__CLASS__) {
     return empty(self::$instance) ? self::$instance=new $class : self::$instance; }
 ///////////
 
 public $serwer, $nazwa, $uzytkownik, $haslo;
 private $idPolaczenia, $iloscZapytan=0;
 
 function polacz()
 {
     if( !( $this->idPolaczenia = mysql_connect( $this->serwer, $this->uzytkownik, $this->haslo ) ) )
         return false;
 
     if( !( mysql_select_db( $this->nazwa, $this->idPolaczenia ) ) )
         return false;
  
     return true;
 }
 
 function rozlacz() {
     mysql_close( $this->idPolaczenia );
 }
 
 function modyfikujBaze( $trescZapytania )
 {
     $this->iloscZapytan++;
  
  //   echo $trescZapytania;
  
     if( $wynikZapytania = mysql_query( $trescZapytania, $this->idPolaczenia ) )
        return $wynikZapytania;
         else return false;
           
 }
 
 function czytajBaze( $trescZapytania )
 {
     if( !($wynikZapytania = $this->modyfikujBaze( $trescZapytania )) )
         return false;
 
     while ($row = mysql_fetch_array( $wynikZapytania ))
           $table[] = $row;
  
     @mysql_free_result( $wynikZapytania );
  
     return $table;
 }
}
 
class Uzytkownik
{
 //singleton
 protected static $instance=null;
 protected function __construct() {}
 public static function getInstance($class=__CLASS__) {
     return empty(self::$instance) ? self::$instance=new $class : self::$instance; }
 ///////////
 
 public $imie, $nazwisko, $login;
 private $id, $haslo;
 
 function wybierzPozycje( $id )
 {
     $id = filtrMysql( $id );
 
     if( !(is_numeric($id)) )
         return false;
 
     if( $wynik = bazaDanych::getInstance()->czytajBaze( "SELECT * FROM uzytkownicy WHERE id=$id" ) )
     {
         $this->id = $id;
      
         $this->login -> $wynik[1][2];
         $this->haslo -> $wynik[1][3];
         $this->imie -> $wynik[1][4];
         $this->nazwisko -> $wynik[1][5];
 
         return true;
     }
     else
         return false;
 }
 
 function ustawHaslo( $haslo )
 {
     if( !(dlugosc($haslo,8,20)) )
         return false;
     $haslo = filtrMysql( $haslo );
  
     $this->haslo = sha1( $haslo );
     return true;
 }
 
 function dodaj()
 {
     if( empty( $id ) & $this->sprawdzDane() )
         return bazaDanych::getInstance()->modyfikujBaze( "INSERT INTO uzytkownicy (id, login, haslo, imie, nazwisko) VALUES ('', '$this->login', '$this->haslo', '$this->imie', '$this->nazwisko')" );
     return false;
 }
 
 function usun()
 {
     if ( !empty( $id ) )
         return bazaDanych::getInstance()->modyfikujBaze( "DELETE FROM uzytkownicy WHERE id=$this->id" );
     else
         return false;
 }
 
 function edytuj()
 {
     if( !empty( $id ) & $this->sprawdzDane() )
         return bazaDanych::getInstance()->modyfikujBaze( "UPDATE uzytkownicy SET login='$this->login', haslo='$this->haslo', imie='$name->imie', nazwisko='$nazwisko->nazwisko' WHERE id=$this->id" );
     return false;
 }
 
 function wczytajWszystkich()
 {
     return bazaDanych::getInstance()->czytajBaze( "SELECT * FROM uzytkownicy" );
 }
 
 function sprawdzDane()
 {
     $this->login = filtrMysql( $this->login );
     $this->imie = filtrMysql( $this->imie );
     $this->nazwisko = filtrMysql( $this->nazwisko );
 
     if( dlugosc($this->login,3,15) & !empty($this->haslo) & dlugosc($this->imie,3,15) & dlugosc($this->nazwisko,3,15) );
         return true;
     return false;
 }
}
 
function dlugosc( $zmienna, $min, $max )
{
 if( strlen( $zmienna ) >= $min & strlen( $zmienna ) <= $max )
     return true;
 return false;
}
function filtrMysql( $zmienna )
{
 if (get_magic_quotes_gpc())
     $zmienna = stripslashes($zmienna);
 $zmienna = mysql_real_escape_string($zmienna);
 return($zmienna);
}
?>
[PHP] pobierz, plaintext 

[PHP] pobierz, plaintext 
<?php
$sql = bazaDanych::getInstance();
 
$sql->serwer = 'localhost';
$sql->nazwa = 'aaa';
$sql->uzytkownik = 'aaa';
$sql->haslo = 'aaa';
 
$sql->polacz();
 
Uzytkownik::getInstance()->imie="Jan";
Uzytkownik::getInstance()->login="jan12";
Uzytkownik::getInstance()->nazwisko="Kowalski";
Uzytkownik::getInstance()->ustawHaslo("passs");
Uzytkownik::getInstance()->dodaj();
 
bazaDanych::getInstance()->rozlacz();
?>
[PHP] pobierz, plaintext 

Ten post edytował marekc12 27.06.2009, 21:45:12