Pomoc - Szukaj - Użytkownicy - Kalendarz
Pełna wersja: Ocena podejścia do Dependency Injection
Forum PHP.pl > Forum > PHP > Object-oriented programming
marcinq123
Witam, uczę się OOP, zacząłem dla testów pisać klasę obsługująca bazę na podstawie PDO ( Trudno to nazwać DIC ). I proszę Was o ocenę czy w dobrą stronę to zmierza czy może podejście mam złe i później sobie skomplikuje życie ?

"Klasa DIC"

  1. class DataBaseConnection
  2. {
  3. protected $pdo;
  4. protected $mysqlHost = 'Host';
  5. protected $mysqlLogin = 'Login';
  6. protected $mysqlPassword = 'Haslo do bazy';
  7. protected $mysqlDatabase = 'Baza';
  8. public function __construct()
  9. {
  10.  
  11. try
  12. {
  13. $this->pdo = new PDO('mysql:dbname='.$this->mysqlDatabase.';host='.$this->mysqlHost.';', $this->mysqlLogin, $this->mysqlPassword);
  14. $this->pdo->setAttribute(PDO::ATTR_DEFAULT_FETCH_MODE, PDO::FETCH_ASSOC);
  15.  
  16. }
  17. catch(PDOException $exception)
  18. {
  19. echo $exception->getMessage();
  20. }
  21. }
  22.  
  23. public function prepare($sql)
  24. {
  25. return $this->pdo->prepare($sql);
  26. }
  27.  
  28. }


Pomijając kwestię iż trzymam dane do połączenia w klasie, czy ma mniej więcej tak to wyglądać ? Ciekawi mnie dlaczego musiałem zdefiniować funkcję prepare ... jak jej nie dopiszę to wywala mi później że nie istnieje ;x

Klasa User:

  1. class User
  2. {
  3. private $login;
  4. private $query;
  5. public function __construct($baza)
  6. {
  7.  
  8. $this->pdo = $baza;
  9.  
  10. }
  11.  
  12. public function selectUser($user)
  13. {
  14. $this->login = $user;
  15.  
  16. $this->query = $this->pdo->prepare('SELECT * FROM users WHERE login =:login');
  17. $this->query->bindParam(':login', $this->login, PDO::PARAM_STR);
  18. $this->query->execute();
  19.  
  20. if($this->query->rowCount() > 0)
  21. {
  22.  
  23. $row=$this->query->fetch();
  24. $this->nazwa = $row['data'];
  25. }else{echo ' dupa ';}
  26. }
  27.  
  28.  
  29.  
  30. public function getUserData()
  31. {
  32.  
  33. return $this->nazwa;
  34.  
  35. }
  36.  
  37. }


Kod napisany tylko żeby sprawdzić czy się uruchamia.

A wywołuje metody tak:

  1. $user = new User(new DataBaseConnection());
  2.  
  3. $user->selectUser('jakisUser');
  4. echo $user->getUserData();


Ogólnie działa, no chyba że skasuje metode prepare w klasie DataBaseConnection to wtedy już nie ;p Tylko pytanie jak już wcześniej napisałem czy dobrą drogą idę ?
Turson
Pokaż var_dump($baza) w konstruktorze klasy User. Siłą rzeczy nie powinieneś mieć dostępu do własności $pdo klasy DataBaseConnection bo jest ona chroniona
Damonsson
Takie trochę na siłę to DI. Do każdego obiektu będziesz musiał przekazywać to połączenie, wg mnie bez sensu. A to raczej sam model sobie powinien dziedziczyć po DataBaseConnection i tyle.
marcinq123
Cytat(Turson @ 5.01.2015, 21:38:56 ) *
Pokaż var_dump($baza) w konstruktorze klasy User. Siłą rzeczy nie powinieneś mieć dostępu do własności $pdo klasy DataBaseConnection bo jest ona chroniona


  1. object(DataBaseConnection)#2 (5) { ["pdo":protected]=> object(PDO)#3 (0) { } ["mysqlHost":protected]=> string(13) "Host" ["mysqlLogin":protected]=> string(8) "Login" ["mysqlPassword":protected]=> string(16) "Haslo do bazy" ["mysqlDatabase":protected]=> string(8) "Baza" }


Cytat(Damonsson @ 5.01.2015, 21:45:58 ) *
Takie trochę na siłę to DI. Do każdego obiektu będziesz musiał przekazywać to połączenie, wg mnie bez sensu. A to raczej sam model sobie powinien dziedziczyć po DataBaseConnection i tyle.


Chciałem sobie uprościć... W tym przypadku extend rozumiem. Ale dajmy na to, chciałbym stworzyć klasę rejestracyjną, logowanie, obsługa profilu klienta, panel admina ... etc. Miałbym je po prostu dziedziczyć po klasie obsługującej bazę ?
Bo chyba nie Sigleton'em miałbym to rozwiązać ?

Broń Boże się nie wymądrzam, po prostu nie jestem pewien jak najlepiej rozwiązać operacje na bazie ... Albo inaczej, jak najszybciej załapać.
Turson
Właśnie Singleton się do tego nadaje.
marcinq123
No właśnie, ogólnie sobie poczytałem ... i w sumie zdania są podzielone ... sporo osób odradza singletona ... albo odradza używania źle napisanego singletona ;p
Pomyślałem nad właśnie DI ...
Damonsson
Jeżeli to jest po prostu zwykły model, to musisz mieć połączenie z bazą, więc po wymuszać przekazywanie czegoś, bez czego jak nie przekażesz to nie będzie działać? A skoro musisz to przekazać to takie użycie DI, jest wg mnie średnim pomysłem.
Dokładnie, miałbyś je po prostu dziedziczyć, użyć Singleton i tyle. Czasami ludzie próbują na siłę doszukiwać się problemów, z tymi wadami Singleton.
Crozin
Z Twoim kodem wszystko jest źle i nie ma to wiele wspólnego z Dependency Injection - niestety podobnie jest z późniejszymi postami w tym wątku.

1. Po co istnieje klasa DataBaseConnection skoro nie robi ona absolutnie nic poza obcięciem możliwości "surowego" PDO? Przecież możesz bezpośrednio korzystać z obiektu PDO w swoim kodzie - nie musisz tego robić przez obiekt pośredniczący.
2. Jak już, to konstruktor klasy DataBaseConnection powinien przyjąć obiekt PDO w swoim argumencie, a nie tworzyć go samemu - wtedy mielibyśmy do czynienia właśnie z DI.
3. Jeżeli nie jesteś wstanie zrobić nic z wyjątkiem, to go nie wyłapuj.
4. Musiałeś zdefiniować dla klasy DataBaseConnection metodę prepare bo dlaczego niby miałaby ona istnieć sama z siebie? Obiekt PDO udostępnia prepare(), query(), beginTransaction() i masę innych, nie DataBaseConnection.
5. W klasie User już poprawnie przekazałeś obiekt połączenia z zewnątrz. Jedynie niepotrzebnie na siłę wykorzystujesz właściwości login czy query bo tam możesz skorzystać ze zwykłych zmiennych lokalnych.

Czyli generalnie powinno to wyglądać to mniej-więcej tak:
  1. // Klasa DataBaseConnection w ogóle do kosza.
  2.  
  3. class User
  4. {
  5. private $pdo;
  6.  
  7. private $name;
  8.  
  9. public function __construct(PDO $pdo)
  10. {
  11. $this->pdo = $pdo;
  12. }
  13.  
  14. public function select($login)
  15. {
  16. $stmt = $this->pdo->prepare('SELECT ... ');
  17. $stmt->bind(..., $login);
  18. $stmt->execute();
  19.  
  20. if ($stmt->rowCount() === 0) {
  21. throw new ...Exception('...');
  22. }
  23.  
  24. $data = $stmt->fetch();
  25.  
  26. $this->name = $data['name'];
  27. }
  28.  
  29. public function getName()
  30. {
  31. return $this->name;
  32. }
  33. }
Pomijam tutaj już fakt, że wybieranie danych z bazy i ich reprezentacja w formie jednego obiektu klasy User jest bardzo złym podejściem. Powinieneś mieć raczej jeden obiekt przeznaczony do reprezentowania użytkownika (User) i drugi do wybierania takowych np. z bazy danych, czyli tzw. "model" (UserRepository).

Jeszcze co do bzdur wypisanych w postach:
1. W żadnym wypadku klasa typu user nie powinna dziedziczyć po połączeniu z bazą danych, bo: (a) nie jest takowym, (cool.gif dojdzie do tego że będziesz miał otwartych po kilka połączeń na raz - kompletnie niepotrzebnie.
2. Jeżeli w jakiejś klasie potrzebujesz mieć dostęp do bazy danych to obiekt PDO powinieneś właśnie przekazać w konstruktorze.
3. Użycie Singetonu dla "klasy od bazy danych" to idiotyzm: (a) przecież może istnieć wiele równoległych połączeń do różnych baz danych(exclamation.gif!), (cool.gif niweczysz tym wszelkie zalety płynące z IoC (DI)
4. Model nie jest synonimem operacji na bazie danych. Ta ostatnia jest w ogóle nie potrzebna. Przecież dane mogą pochodzić z dziesiątek innych źródeł, a tak w ogóle to model nie oznacza tylko wybierania danych, ale również i ich przetwarzanie.
pyro
@Crozin, +1 . Miałem to samo wymienić, ale nie chciało mi się tyle pisać biggrin.gif
marcinq123
Prawdę mówiąc, po cichu liczyłem na Twoją odpowiedź Crozin ;]

Z racji tego iż chcę nabrać dobrych nawyków, spróbuje podejść kompleksowo.

Cytat
4. Musiałeś zdefiniować dla klasy DataBaseConnection metodę prepare bo dlaczego niby miałaby ona istnieć sama z siebie? Obiekt PDO udostępnia prepare(), query(), beginTransaction() i masę innych, nie DataBaseConnection.

No właśnie zdefiniowałem tylko prepare() a np. execute(), bindParam() już nie musiałem bo działało ... i dlatego nie rozumiem czemu.


Cytat
Pomijam tutaj już fakt, że wybieranie danych z bazy i ich reprezentacja w formie jednego obiektu klasy User jest bardzo złym podejściem. Powinieneś mieć raczej jeden obiekt przeznaczony do reprezentowania użytkownika (User) i drugi do wybierania takowych np. z bazy danych, czyli tzw. "model" (UserRepository).


Cenna uwaga, ale napisałem przykład z pierwszego postu tylko żeby sprawdzić czy w ogóle to działa.

  1. // Klasa DataBaseConnection w ogóle do kosza.
  2.  
  3. class User
  4. {
  5. private $pdo;
  6.  
  7. private $name;
  8.  
  9. public function __construct(PDO $pdo)
  10. {
  11. $this->pdo = $pdo;
  12. }
  13.  
  14. public function select($login)
  15. {
  16. $stmt = $this->pdo->prepare('SELECT ... ');
  17. $stmt->bind(..., $login);
  18. $stmt->execute();
  19.  
  20. if ($stmt->rowCount() === 0) {
  21. throw new ...Exception('...');
  22. }
  23.  
  24. $data = $stmt->fetch();
  25.  
  26. $this->name = $data['name'];
  27. }
  28.  
  29. public function getName()
  30. {
  31. return $this->name;
  32. }
  33.  
  34. //oraz inne metody
  35. }


Czyli teraz odwołuje się tak ?:


  1. $user = new User(new PDO('mysql:dbname=base;host=host;', 'user', 'password'));
  2. $user->select('user');
  3. echo $user->getName();


Moje pytanie brzmi, dajmy na to miałbym klasy User ... Admin, Auth, etc. ( nie wiem co tam jeszcze wymyśle tongue.gif)
Każdy obiekt generuje ( w razie potrzeby połączenia z bazą) poprzez

  1. $admin= new Admin(new PDO('mysql:dbname=base;host=host;', 'user', 'password'));
  2. //jakieś wywoływane metody


A czy takie podejście żeby się nie powtarzać ma sens?
  1. $connection = new PDO('mysql:dbname=base;host=host;', 'user', 'password'));
  2. $admin= new Admin($connection);





A do reszty się nie ustosunkuje bo ze wstydu nie wiem nawet co powiedzieć oneeyedsmiley02.png
Crozin
Cytat
No właśnie zdefiniowałem tylko prepare() a np. execute(), bindParam() już nie musiałem bo działało ... i dlatego nie rozumiem czemu.
Twoja metoda DataBaseConnection::prepare() zwraca na dobrą sprawę to co zwraca PDO::prepare() czyli nowy obiekt, klasy PDOStatement - to on zawiera metody execute(), bindParam() i inne. Nie ma on już kompletnie nic wspólnego z Twoim kodem.

Cytat
A czy takie podejście żeby się nie powtarzać ma sens?
Tak, ma to sens - tak właśnie powinno to wyglądać. Bo tutaj nie chodzi nawet o powtarzanie się, a o jednokrotne nawiązanie połączenia i wykorzystywanie go w wielu różnych miejscach, czyli:
  1. $conn = new PDO(....);
  2.  
  3. $user = new User($conn);
  4. $sth = new Sth($conn);
  5. $userSth = new UserSth($conn, $user);
  6. // itp.
marcinq123
Na tą chwile zostały rozwiane moje wątpliwości ;]

Dziękuje wielkie za pomoc panowie.
Damonsson
Cytat(Crozin @ 6.01.2015, 12:41:19 ) *
1. W żadnym wypadku klasa typu user nie powinna dziedziczyć po połączeniu z bazą danych, bo: (a) nie jest takowym, (cool.gif dojdzie do tego że będziesz miał otwartych po kilka połączeń na raz - kompletnie niepotrzebnie.
2. Jeżeli w jakiejś klasie potrzebujesz mieć dostęp do bazy danych to obiekt PDO powinieneś właśnie przekazać w konstruktorze.
3. Użycie Singetonu dla "klasy od bazy danych" to idiotyzm: (a) przecież może istnieć wiele równoległych połączeń do różnych baz danych(exclamation.gif!), (cool.gif niweczysz tym wszelkie zalety płynące z IoC (DI)
4. Model nie jest synonimem operacji na bazie danych. Ta ostatnia jest w ogóle nie potrzebna. Przecież dane mogą pochodzić z dziesiątek innych źródeł, a tak w ogóle to model nie oznacza tylko wybierania danych, ale również i ich przetwarzanie.

1. Jeżeli to jest zwykły model do połączenia z bazą danych to jest takowym i w każdym obiekcie i tak będzie się łączył z bazą danych i duplikował przekazywanie połączenia w konstruktorze przecież. Po to ma użyć Singleton, żeby nie miał kilku otwartych połączeń.
2. Jasne.
3. Tak sobie przeglądam klasy Doctrine zawierające własnie Singleton np w getInstance i nie widzę idiotyzmu.
4. Jasne, jeżeli zakładamy, że to ma być uniwersalny model, zgadzam się. Tutaj było pytanie o PDO.

Nie piszę po złości, masz wiedzę większą ode mnie i chętnie poczytam, dlaczego się mylę.

Choć te różnice nie są duże, głównie chodzi o to, że moje założenie jest takie, że zawsze będzie się łączył z tą bazą danych, więc może dziedziczyć, a Ty dajesz większą uniwersalność, w zamian za duplikowanie kodu (przekazywanie połączenia za każdy razem, przy tworzeniu obiektu). Pytanie, co jest poprawniejsze?
Crozin
Cytat
1. Jeżeli to jest zwykły model do połączenia z bazą danych to jest takowym i w każdym obiekcie i tak będzie się łączył z bazą danych i duplikował przekazywanie połączenia w konstruktorze przecież. Po to ma użyć Singleton, żeby nie miał kilku otwartych połączeń.
Co to jest "zwykły model"? Jeżeli przekazuje za każdym razem to samo połączenie to nie nawiąże mu się nagle kilka.
Cytat
3. Tak sobie przeglądam klasy Doctrine zawierające własnie Singleton np w getInstance i nie widzę idiotyzmu.
A jakoś dokładniej? Bo na pewno nie jest to obiekt połączenia z bazą danych.
Cytat
[...] w zamian za duplikowanie kodu (przekazywanie połączenia za każdy razem, przy tworzeniu obiektu)
Ale tu nie ma żadnego duplikowania kodu.
Damonsson
1. Metody, które i tak zawsze pobierają coś z bazy danych, nie wyobrażam tam sobie nagle czegoś niezwiązanego z bazą danych. Wtedy trzeba by to rozdzielić. No tak, a jeżeli będę dziedziczył (wg mojej teorii), to muszę sprawdzić czy już nie istnieje połączenie dlatego Singleton.

2. Dalej w związku z tym jest tworzenie połączenia itd. ale właśnie samo getInstance to zwykły Singleton.
  1. /**
  2.   * Returns an instance of this class
  3.   * (this class uses the singleton pattern)
  4.   *
  5.   * @return Doctrine_Manager
  6.   */
  7. public static function getInstance()
  8. {
  9. if ( ! isset(self::$_instance)) {
  10. self::$_instance = new self();
  11. }
  12. return self::$_instance;
  13. }


3. Za każdym razem musisz się martwić, żeby przekazać to połączenie do obiektu. Tworząc odpowiednio skonkretyzowany model, wiem, że będzie on korzystał z bazy danych i w konstruktorze, tworzę sobie to połączenie od razu. Tworząc obiekt nie muszę pamiętać, aby przekazać tam połączenie z bazą.
Crozin
Doctrine 1? Ale mamy 2015 rok, środowisko PHP już znormalniało...
O niczym nie musisz pamiętać, IDE/PHP samo Ci podpowie, że robisz coś nieprawidłowego.
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.