Pomoc - Szukaj - Użytkownicy - Kalendarz
Pełna wersja: Oceńcie czy kod jest ok
Forum PHP.pl > Forum > PHP
marekc12
  1. <?php
  2. class bazaDanych
  3.    {
  4.        private $serwer, $nazwa, $uzytkownik, $haslo, $idPolaczenia, $wynikZapytania;
  5.        
  6.        function __construct( $serwer, $nazwa, $uzytkownik, $haslo )
  7.        {
  8.            $this->serwer = $serwer;
  9.            $this->nazwa = $nazwa;
  10.            $this->uzytkownik = $uzytkownik;
  11.            $this->haslo = $haslo;
  12.            
  13.            $this->idPolaczenia = mysql_connect( $this->serwer, $this->uzytkownik, $this->haslo );
  14.            if( !$this->idPolaczenia )
  15.                $this->blad();
  16.  
  17.            $dbSelected = mysql_select_db( $this->nazwa, $this->idPolaczenia );
  18.            if( !$dbSelected )
  19.                $this->blad();
  20.        }
  21.  
  22.        function __destruct() {
  23.            mysql_close( $this->idPolaczenia );
  24.        }
  25.        
  26.        function modyfikujBaze( $trescZapytania )
  27.        {
  28.            $this->wynikZapytania = mysql_query( $trescZapytania, $this->idPolaczenia )
  29.            or $this->blad( $trescZapytania );
  30.        }
  31.        
  32.        function czytajBaze( $trescZapytania )
  33.        {
  34.            $this->modyfikujBaze( $trescZapytania );
  35.            
  36.            while ($row = mysql_fetch_array( $this->wynikZapytania ))
  37.                $table[] = $row;
  38.            
  39.            return $table;
  40.        }
  41.  
  42.        private function blad( $trescZapytania ) {
  43.            die( mysql_errno( $this->idPolaczenia ) . ': ' . mysql_error( $this->idPolaczenia ) );
  44.        }
  45.    }
  46.  
  47.    class uzytkownik
  48.    {
  49.        private $bazaDanych, $id, $uprawnienia, $login, $haslo, $imie;
  50.        
  51.        function __construct( $bazaDanych, $id, $uprawnienia, $login, $haslo, $imie )
  52.        {
  53.            $this->bazaDanych = $bazaDanych;
  54.            $this->id = $id;
  55.            $this->uprawnienia = $uprawnienia;
  56.            $this->login = $login;
  57.            $this->haslo = $haslo;
  58.            $this->imie = $imie;
  59.        }
  60.        
  61.        function dodaj() {
  62.            $this->bazaDanych -> modyfikujBaze( "INSERT INTO users (id, rights, login, pass, name) VALUES ('', '$this->uprawnienia', '$this->login', '$this->haslo', '$this->imie')\" );
  63.        }
  64.        
  65.        function usun() {
  66.            $this->bazaDanych -> modyfikujBaze( "DELETE FROM users WHERE id=$this->id&#092;" );
  67.        }
  68.  
  69.        function edytuj() {
  70.            $this->bazaDanych -> modyfikujBaze( "UPDATE users SET rights='$this->uprawnienia', login='$this->login', pass='$this->haslo', login='$name->imie' WHERE id=$this->id\" );
  71.        }
  72.        
  73.        function wczytajPozycje() {
  74.            return $this->bazaDanych -> czytajBaze("SELECT * FROM users WHERE id=$this->id&#092;" );
  75.        }
  76.    }
  77. ?>

  1. <?php
  2. $mySql = new bazaDanych( 'localhost', 'lala', 'asdas', 'asdkl' );
  3. $osoba = new uzytkownik( $mySql, null, 1, 'login123', 'mojehaslo', 'marek' );
  4. $osoba -> dodaj();
  5. ?>


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

  1. <?php
  2. class Uzytkownik
  3. {
  4. //singleton
  5.    private static $oInstance = false;
  6.   private function __construct() {}
  7.  
  8.    public static function getInstance()
  9.    {
  10.        if( self::$oInstance == false )
  11.            self::$oInstance = new Singleton();
  12.        return self::$oInstance;
  13.    }
  14. //koniec: singleton
  15.  
  16.    public $imie, $nazwisko;
  17.    private $id = null, $login = null, $haslo = null;
  18.    
  19.    function wybierzPozycje( $id )
  20.    {
  21.        $id = filtrMyql( $id );
  22.        
  23.        // jesli istnieje uzytkownik o takim ID wczytaj go z bazy        
  24.        if( $wynik = czytajBaze( &#092;"SELECT * FROM uzytkownicy WHERE id=$id\" ) )
  25.        {
  26.            $this->id = $id;
  27.            
  28.            $this->login -> $wynik[1][2];
  29.            $this->haslo -> $wynik[1][3];
  30.            $this->imie -> $wynik[1][4];
  31.            $this->nazwisko -> $wynik[1][5];
  32.            
  33.            return $wynik;
  34.        }
  35.        else
  36.            return false;
  37.    }
  38.    
  39.    function ustawLogin( $login )
  40.    {
  41.        //jesli uzytkownika nie ma jeszcze w bazie( nie mamy ustawionego ID) to zmieniamy login
  42.        if ( $id )
  43.        {
  44.            $this->login = filtrMyql( $login );
  45.            return true;
  46.        }
  47.        else
  48.            return false;
  49.    }
  50.    
  51.    function ustawHaslo( $haslo )
  52.    {
  53.        //szyfrujemy haslo    
  54.        $this->haslo = md5( filtrMyql( $haslo ) );
  55.    }
  56.    
  57.    function dodaj()
  58.    {
  59.        $imie = filtrMyql( $imie );
  60.        $nazwisko = filtrMyql( $nazwisko );
  61.        
  62.        //tu jeszcze trzeba dopisać sprawdzanie czy login nie jest zajęty
  63.            
  64.        if( ! modyfikujBaze( &#092;"INSERT INTO uzytkownicy (id, login, haslo, imie, nazwisko) VALUES ('', '$this->login', '$this->haslo', '$this->imie', '$this->nazwisko')\" ) )
  65.            return false;
  66.    }
  67.    
  68.    function usun()
  69.    {
  70.        if( ! modyfikujBaze( &#092;"DELETE FROM uzytkownicy WHERE id=$this->id\" ) )
  71.            return false;
  72.    }
  73.    
  74.    function edytuj()
  75.    {
  76.        $imie = filtrMyql( $imie );
  77.        $nazwisko = filtrMyql( $nazwisko );
  78.        
  79.        if( ! modyfikujBaze( &#092;"UPDATE uzytkownicy SET haslo='$this->haslo', imie='$name->imie', imie='$nazwisko->nazwisko' WHERE id=$this->id\" ) )
  80.            return false;
  81.    }
  82.    
  83.    function wczytajWszystkich()
  84.    {
  85.        $wynik = czytajBaze( &#092;"SELECT * FROM uzytkownicy\" ) )
  86.        return $wynik;
  87.    }
  88. }
  89. ?>
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.
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(); 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ł
  1. <?php
  2. protected static $instance=null;
  3. protected function __construct() {}
  4.  
  5. public static function getInstance($class=__CLASS__)
  6. {
  7.   return empty(self::$instance) ? self::$instance=new $class : self::$instance;
  8. }
  9. ?>


ponieważ można kopiować prędko do każdej klasy bez potrzeby pisania zmian 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):



  1. <?php
  2. function modyfikujBaze( $trescZapytania )
  3.        {
  4.            $wynikZapytania = mysql_query( $trescZapytania, $this->idPolaczenia )
  5.  return $wynikZapytania;          
  6.        }
  7.        
  8.        function czytajBaze( $trescZapytania )
  9.        {
  10.  $wynikZapytania = modyfikujBaze( $trescZapytania );
  11.            
  12.            while ($row = mysql_fetch_array( $wynikZapytania ))
  13.                $table[] = $row;
  14.            
  15.            return $table;
  16.        }
  17. ?>
em1X
te nazwy to z kosmosu chyba bierzesz smile.gif

  1. <?php
  2. function execute($sql)
  3. {
  4.   $rs=@mysql_query($sql, $this->idPolaczenia);
  5.   if (mysql_errno($this->idPolaczenia)) {
  6.      throw new Exception($sql . ': ' . mysql_error(), mysql_errno());
  7.   }
  8.  
  9.   return $rs;
  10. }
  11.  
  12. function & query($sql)
  13. {
  14.   $rs=$this->execute($sql);
  15.   $tab=$row=array();
  16.  
  17.   while($row=mysql_fetch_array($rs)) {
  18.      $tab[]=$row;
  19.   }
  20.  
  21.   return $tab;
  22. }
  23. ?>
marekc12
ok, dzięki za rady 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 smile.gif) Pozdrawiam!



  1. <?php
  2. class bazaDanych
  3. {
  4.  //singleton
  5.  protected static $instance=null;
  6.  protected function __construct() {}
  7.  public static function getInstance($class=__CLASS__) {
  8.      return empty(self::$instance) ? self::$instance=new $class : self::$instance; }
  9.  ///////////
  10.  
  11.  public $serwer, $nazwa, $uzytkownik, $haslo;
  12.  private $idPolaczenia, $iloscZapytan=0;
  13.  
  14.  function polacz()
  15.  {
  16.      if( !( $this->idPolaczenia = mysql_connect( $this->serwer, $this->uzytkownik, $this->haslo ) ) )
  17.          return false;
  18.  
  19.      if( !( mysql_select_db( $this->nazwa, $this->idPolaczenia ) ) )
  20.          return false;
  21.   
  22.      return true;
  23.  }
  24.  
  25.  function rozlacz() {
  26.      mysql_close( $this->idPolaczenia );
  27.  }
  28.  
  29.  function modyfikujBaze( $trescZapytania )
  30.  {
  31.      $this->iloscZapytan++;
  32.   
  33.   //   echo $trescZapytania;
  34.   
  35.      if( $wynikZapytania = mysql_query( $trescZapytania, $this->idPolaczenia ) )
  36.         return $wynikZapytania;
  37.          else return false;
  38.            
  39.  }
  40.  
  41.  function czytajBaze( $trescZapytania )
  42.  {
  43.      if( !($wynikZapytania = $this->modyfikujBaze( $trescZapytania )) )
  44.          return false;
  45.  
  46.      while ($row = mysql_fetch_array( $wynikZapytania ))
  47.            $table[] = $row;
  48.   
  49.      @mysql_free_result( $wynikZapytania );
  50.   
  51.      return $table;
  52.  }
  53. }
  54.  
  55. class Uzytkownik
  56. {
  57.  //singleton
  58.  protected static $instance=null;
  59.  protected function __construct() {}
  60.  public static function getInstance($class=__CLASS__) {
  61.      return empty(self::$instance) ? self::$instance=new $class : self::$instance; }
  62.  ///////////
  63.  
  64.  public $imie, $nazwisko, $login;
  65.  private $id, $haslo;
  66.  
  67.  function wybierzPozycje( $id )
  68.  {
  69.      $id = filtrMysql( $id );
  70.  
  71.      if( !(is_numeric($id)) )
  72.          return false;
  73.  
  74.      if( $wynik = bazaDanych::getInstance()->czytajBaze( "SELECT * FROM uzytkownicy WHERE id=$id" ) )
  75.      {
  76.          $this->id = $id;
  77.       
  78.          $this->login -> $wynik[1][2];
  79.          $this->haslo -> $wynik[1][3];
  80.          $this->imie -> $wynik[1][4];
  81.          $this->nazwisko -> $wynik[1][5];
  82.  
  83.          return true;
  84.      }
  85.      else
  86.          return false;
  87.  }
  88.  
  89.  function ustawHaslo( $haslo )
  90.  {
  91.      if( !(dlugosc($haslo,8,20)) )
  92.          return false;
  93.      $haslo = filtrMysql( $haslo );
  94.   
  95.      $this->haslo = sha1( $haslo );
  96.      return true;
  97.  }
  98.  
  99.  function dodaj()
  100.  {
  101.      if( empty( $id ) & $this->sprawdzDane() )
  102.          return bazaDanych::getInstance()->modyfikujBaze( "INSERT INTO uzytkownicy (id, login, haslo, imie, nazwisko) VALUES ('', '$this->login', '$this->haslo', '$this->imie', '$this->nazwisko')" );
  103.      return false;
  104.  }
  105.  
  106.  function usun()
  107.  {
  108.      if ( !empty( $id ) )
  109.          return bazaDanych::getInstance()->modyfikujBaze( "DELETE FROM uzytkownicy WHERE id=$this->id" );
  110.      else
  111.          return false;
  112.  }
  113.  
  114.  function edytuj()
  115.  {
  116.      if( !empty( $id ) & $this->sprawdzDane() )
  117.          return bazaDanych::getInstance()->modyfikujBaze( "UPDATE uzytkownicy SET login='$this->login', haslo='$this->haslo', imie='$name->imie', nazwisko='$nazwisko->nazwisko' WHERE id=$this->id" );
  118.      return false;
  119.  }
  120.  
  121.  function wczytajWszystkich()
  122.  {
  123.      return bazaDanych::getInstance()->czytajBaze( "SELECT * FROM uzytkownicy" );
  124.  }
  125.  
  126.  function sprawdzDane()
  127.  {
  128.      $this->login = filtrMysql( $this->login );
  129.      $this->imie = filtrMysql( $this->imie );
  130.      $this->nazwisko = filtrMysql( $this->nazwisko );
  131.  
  132.      if( dlugosc($this->login,3,15) & !empty($this->haslo) & dlugosc($this->imie,3,15) & dlugosc($this->nazwisko,3,15) );
  133.          return true;
  134.      return false;
  135.  }
  136. }
  137.  
  138. function dlugosc( $zmienna, $min, $max )
  139. {
  140.  if( strlen( $zmienna ) >= $min & strlen( $zmienna ) <= $max )
  141.      return true;
  142.  return false;
  143. }
  144. function filtrMysql( $zmienna )
  145. {
  146.      $zmienna = stripslashes($zmienna);
  147.  $zmienna = mysql_real_escape_string($zmienna);
  148.  return($zmienna);
  149. }
  150. ?>


  1. <?php
  2. $sql = bazaDanych::getInstance();
  3.  
  4. $sql->serwer = 'localhost';
  5. $sql->nazwa = 'aaa';
  6. $sql->uzytkownik = 'aaa';
  7. $sql->haslo = 'aaa';
  8.  
  9. $sql->polacz();
  10.  
  11. Uzytkownik::getInstance()->imie="Jan";
  12. Uzytkownik::getInstance()->login="jan12";
  13. Uzytkownik::getInstance()->nazwisko="Kowalski";
  14. Uzytkownik::getInstance()->ustawHaslo("passs");
  15. Uzytkownik::getInstance()->dodaj();
  16.  
  17. bazaDanych::getInstance()->rozlacz();
  18. ?>
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.