Pomoc - Szukaj - Użytkownicy - Kalendarz
Pełna wersja: Klasa i PDO prośba o sprawdzenie jakości kodu
Forum PHP.pl > Forum > PHP > Object-oriented programming
Robert3d
Witam
Chciał bym prosić użytkowników o sprawdzenie mojej pracy nad klasą która łączy się z bazą wszystko działa to fakt ale ni wydaje mi się by to był elegancji sposób obsługi bo... PDO bindowanie trzeba wprowadzać oddzielnie i nie wiem jak to zmienić... A pozatym pewnie miało optymalna klasa?

  1. class laczenieZBazaDanych {
  2.  
  3. /// Zmienne dla polaczenia z bazą danych
  4. private $host;
  5. private $nazwaBazyDanych;
  6. private $login;
  7. private $haslo;
  8. /// Baza danych
  9. private $dbh;
  10. private $wynik;
  11.  
  12. public function __construct($host = "localhost", $nazwaBazyDanych = "test", $login = "root" , $haslo = "") {
  13.  
  14. /// Sprawdzamy dane wejsciowe
  15. if (
  16. isset ($host) && is_string($host) &&
  17. isset ($nazwaBazyDanych) && is_string($nazwaBazyDanych) &&
  18. isset ($login) && is_string($login) &&
  19. isset ($haslo) && is_string($haslo)
  20.  
  21. ) {
  22. $this->host = (string) $host;
  23. $this->nazwaBazyDanych = (string) $nazwaBazyDanych;
  24. $this->login = (string) $login;
  25. $this->haslo = (string) $haslo;
  26.  
  27. } else {
  28. die('uprawnienia musza byt typu string');
  29.  
  30. }
  31.  
  32. /// łączenie z bazą danych
  33. try{
  34. $this->dbh = new PDO("mysql:host=$this->host;dbname=$this->nazwaBazyDanych", $this->login, $this->haslo, array(PDO::MYSQL_ATTR_INIT_COMMAND => "SET NAMES utf8"));
  35. $this->dbh -> setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
  36.  
  37. } catch (PDOException $e){
  38. echo "Polaczenie z bazą danych nie mogło zostać wykonane: ". $e->getMessage();
  39.  
  40. }
  41.  
  42. }
  43.  
  44. function __toString() {
  45.  
  46. }
  47.  
  48. public function setZakonczPolaczenie(){
  49. $this->dbh = NULL;
  50. }
  51.  
  52. public function getSql($zapytanieSQL, $bingPDO = NULL, $format = NULL) {
  53. if( isset ($zapytanieSQL) && is_string($zapytanieSQL)){
  54. // zapytanie o wynik
  55. $stmt = $this->dbh->prepare($zapytanieSQL);
  56.  
  57. if( isset ($bingPDO) && is_array($bingPDO)){
  58.  
  59. foreach($bingPDO as $key => $value)
  60. {
  61. $stmt->bindParam($key,$value,PDO::PARAM_STR);
  62. }
  63.  
  64. }else{
  65. echo "Drugi parametr powinien być tablicą";
  66. }
  67.  
  68. }
  69.  
  70. $stmt -> execute();
  71.  
  72. // przetwarzanei wynikow otrzymanych z PDO do tablicy lub zmiennej.
  73. $i=0;
  74. foreach($stmt as $key => $value)
  75. {
  76. switch ($format){
  77. case 1:
  78. $value = array_unique($value);
  79. break;
  80. case 2:
  81. $value = array_unique($value);
  82. $value = array_values($value);
  83. break;
  84. default :
  85. break;
  86. }
  87.  
  88. $table[$i++] = $value;
  89. }
  90.  
  91. // jezeli jest tylko jedna tablica to tylko ja wypisz nie wypisuje ze jest to tablica zerowa.
  92. if(!isset ($table[1])){
  93. $this->wyniki = $table[0];
  94.  
  95. }else{
  96. $this->wyniki = $table;
  97.  
  98. }
  99. }
  100.  
  101. function setWynik() {
  102. return $this->wyniki;
  103. }
  104.  
  105. }
  106.  
  107.  
  108. $test = new laczenieZBazaDanych();
  109. $test2 = $test->getSql('SELECT * FROM oferta',array(),2);
  110. print_r($test->setWynik());
  111. ?>
Crozin
Dlaczego nie skorzystasz z "czystego" PDO, które ma lepszy interfejs niż to coś?
  1. $pdo = new PDO(...);
  2.  
  3. $stmt = $pdo->query('SELECT * FROM oferty');
  4. print_r($stmt->fetchAll());
Bo Twój interfejs to jedynie masa ograniczeń i utrudnień (w stylu zwracania pierwszego elementu kolekcji, w przypadku gdy istnieje w niej tylko on, zamiast całej kolekcji).
Robert3d
Chodzi o bezpieczeństwo zwykłym query można zrobić sql injection ? Stąd użycie bindowania co utrudnia sprawę.

czy dobrze rozumuję?
Crozin
W zapytaniu jakie podałeś ciężko o SQL Injection, gdyż nie ma tam żadnych parametrów. Ale przecież PDO ma również całkiem przystępne API dla bindowania:
  1. $stmt = $pdo->prepare('...');
  2. $stmt->execute(array('param1' => 'val1', 'param2' => 'val2'));
  3. print_r($stmt->fetchAll());
by_ikar
To jak chcesz uniknąć sql injection to podpinaj zapytania. Nie musisz sprawdzać wewnątrz funkcji czy dana zmienna została przekazana w parametrze, bo jeżeli nie podasz jej parametru, to albo zwróci wartość domyślną (w twoim wypadku null) albo gdy nie dasz jakiejś domyślnej wartości, to php wygeneruje błąd. Dodatkowo nie wiem jaki sens jest sprawdzania czy zmienna jest stringiem, skoro podczas połączenia masz TRY i gdy się coś nie powiedzie, zwrócony zostanie wyjątek z komunikatem błędu. Przykładowo lepiej żeby twoja klasa jak już wyglądała tak:

Kod
<?php


class laczenieZBazaDanych
{
    /// Baza danych
    private $dbh;
    private $wynik;

    public function __construct($dbhost, $dbname, $login = null, $password = null, $options = array())
    {
        /// łączenie z bazą danych
        try
        {
            $this->dbh = new PDO('mysql:host='.$dbhost.';dbname='.$dbname, $login, $password, $options);            
            $this->dbh->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
        
        }  catch (PDOException $e)
        {
            echo 'Polaczenie z bazą danych nie mogło zostać wykonane: '.$e->getMessage();
        }
        
    }
}

$db = new laczenieZBazaDanych('localhost', 'dbname', 'login', 'password', array(PDO::MYSQL_ATTR_INIT_COMMAND => 'SET NAMES utf8'));
Robert3d
Jako że jest to klasa ma mi służyć dla różnych zapytań o różnej konstrukcji a więc w przyszłości z całą pewnością zapytanie do bazy będzie dość skomplikowane i z wieloma wytycznymi w dodatku często przez tablice GET którą można dowolnie modyfikować sad.gif .

by_ikar - wezmę pod uwagę

mam nadzieję że ktoś jeszcze się wypowie.


OTO NOWA WERSJA mógł by sie ktoś wypowiedzieć?

  1. class interfejsBazy {
  2.  
  3. //Baza danych
  4. private $dbh;
  5. private $wynik;
  6.  
  7. function __construct($dbhost, $dbname, $login = null, $password = null, $options = array(PDO::MYSQL_ATTR_INIT_COMMAND => 'SET NAMES utf8') ) {
  8. /// łączenie z bazą danych
  9. try
  10. {
  11. $this->dbh = new PDO('mysql:host='.$dbhost.';dbname='.$dbname, $login, $password, $options);
  12. $this->dbh->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
  13.  
  14. } catch (PDOException $e)
  15. {
  16. echo 'Polaczenie z bazą danych nie mogło zostać wykonane: '.$e->getMessage();
  17. }
  18. }
  19.  
  20. function query($sqlPDO,$execPDO){
  21.  
  22. $stmt = $this->dbh->prepare($sqlPDO);
  23. $stmt->execute($execPDO);
  24. $this->wynik = $stmt->fetchAll();
  25.  
  26. }
  27.  
  28. function returnQuery(){
  29. return $this->wynik;
  30. }
  31.  
  32. function __destruct() {
  33. $dbh = NULL;
  34. }
  35. }
  36.  
  37. $db = new interfejsBazy("localhost", "test", "root", "");
  38. $zmienna = "2222222";
  39. $db->query('SELECT * FROM oferta WHERE telefon LIKE :zmienna',array(':zmienna' => $zmienna));
  40. print_r($db->returnQuery());
  41. ?>
Crozin
Możesz określić cel istnienia tego tworu? Jakie problemy on rozwiązuje? Albo jakie problemy można dzięki niemu łatwiej rozwiązać?
Bo ja tu widzę jedynie same ograniczenia i problemy na rzecz skrócenia kodu o jakieś 10 znaków w jednym, konkretnym przypadku.

Wypisz może najpierw jakie problemy / ograniczenia stwarza PDO (podpowiem, że nawet jest ich kilka) i jeżeli jakieś faktycznie znajdziesz to wtedy zabierz się za ich rozwiązywanie.
Robert3d
Ogólnie to uczę się pisać obiektowo smile.gif Pisze sobie małe projekciki. Cel jakiś interfejs by z niego korzystać w różnych sytuacjach.

Jeżeli wiesz coś czego ja nie wiem podziel się smile.gif)
Crozin
No to podstawą OOP jest wykorzystywanie istniejących narzędzi.
Cytat
Cel jakiś interfejs by z niego korzystać w różnych sytuacjach.
I taki udostępnia Ci PDO.
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.