Witaj Gościu! ( Zaloguj | Rejestruj )

Forum PHP.pl

 
Reply to this topicStart new topic
> PDO Singleton, Proszę o ocenę
q3trm
post 16.04.2013, 19:18:47
Post #1





Grupa: Zarejestrowani
Postów: 83
Pomógł: 1
Dołączył: 26.02.2013

Ostrzeżenie: (0%)
-----


Witam.


Chciałbym się dowiedzieć czy idę dobrą drogą co_jest.gif

Posiadam dwie klasy, które są odpowiedzialne za nawiązywanie łączności z bazą danych. Klasa _DSN - przygotowuję dane potrzebne konstruktorowi PDO, dane te są zapisane w pliku tekstowym. Druga klasa to _PDOConnect(Singleton), odbiera i nawiązuje połączenie z bazą. Klasy są surowe jeżeli chodzi o sprawdzanie błędów itp. jest to świadome.

  1. <?php
  2.  
  3. class _DSN // Pobiera plik z danymi do logowania PDO
  4. {
  5. private $file;
  6. static $dsnInfo = array();
  7.  
  8. function __construct ($file) //Pobiera ścieżkę pliku i wywołuje metody.
  9. {
  10. if(is_file($file) && file_exists($file))
  11. {
  12. $this ->file = $file;
  13.  
  14. $this ->openFileDSN();
  15. $this ->saveDSNInfo();
  16. }
  17. else {return false;}
  18. }
  19.  
  20. function openFileDSN() // kroi dane DSN z pliku na tablice numeryczną
  21. {
  22. $file_get_contents = file_get_contents($this ->file);
  23. self::$dsnInfo = explode('|' ,$file_get_contents);
  24. }
  25. function saveDSNInfo() // zamienia indeksy numeryczne na string
  26. {
  27. $key = array('driver','host','dbname','user','password');
  28. self::$dsnInfo = array_combine($key, self::$dsnInfo);
  29. }
  30. }
  31.  
  32. class _PDOConnect
  33. {
  34. static private $instance;
  35.  
  36. private function __construct()
  37. {
  38. $Dsn = new _DSN('dsn.txt'); // W celu wykonania metod klasy _Dsn. Parametr konstruktora pobierany zewnątrz klasy.
  39.  
  40. try
  41. {
  42. $PDO = new PDO (
  43. _Dsn::$dsnInfo['driver'].
  44. _Dsn::$dsnInfo['host'].
  45. _Dsn::$dsnInfo['dbname'].
  46. _Dsn::$dsnInfo['user'].
  47. _Dsn::$dsnInfo['password']
  48. );
  49. }
  50. catch (Exception $e)
  51. {
  52. echo $e ->getMessage();
  53. }
  54. }
  55.  
  56. public static function getInstance()
  57. {
  58. if (empty(self::$instance))
  59. {
  60. self::$instance = new _PDOConnect();
  61. }
  62. return self::$instance;
  63. }
  64. }
  65.  
  66.  
  67. ?>
Go to the top of the page
+Quote Post
Pilsener
post 16.04.2013, 21:00:25
Post #2





Grupa: Zarejestrowani
Postów: 1 590
Pomógł: 185
Dołączył: 19.04.2006
Skąd: Gdańsk

Ostrzeżenie: (0%)
-----


1. Jeśli ma to być singleton brakuje prywatnej/chronionej metody __clone
2. Przemyślałbym obsługę wielu baz, bo nawet jeśli tego nie przewidujemy to może taka potrzeba się pojawić. I pytanie, czy musi to być singleton bo przecież nie piszemy skryptu, gdzie w każdym pliku na samym początku DB::getInstance()
3. Współpracę klas oceniam fatalnie - poczytaj o wzorcach współpracy klas, na początek dependency injection, "wstrzykiwanie zależności" - tak to chyba po polsku się nazywa wink.gif
4. Podobnie klasę _DSN - zobacz, jak tego używasz, najpierw tworzysz obiekt by wywołać konstruktor a potem dobierasz się do pól statycznych? smile.gif Podstawą są gettery i settery a nie kombinowanie metodami statycznymi czy magicznymi. A jeśli już to powinieneś pobrać sobie ten cfg jedną metodą statyczną gdzie przekazujesz ten plik jako parametr, bez konieczności tworzenia obiektu.
Go to the top of the page
+Quote Post
Szymciosek
post 16.04.2013, 21:29:22
Post #3





Grupa: Zarejestrowani
Postów: 1 168
Pomógł: 126
Dołączył: 5.02.2010
Skąd: Świdnica

Ostrzeżenie: (0%)
-----


Możesz coś więcej powiedzieć, gdzie tutaj byś zastosował DI?
Go to the top of the page
+Quote Post
emp
post 16.04.2013, 21:54:12
Post #4





Grupa: Zarejestrowani
Postów: 195
Pomógł: 14
Dołączył: 12.01.2006
Skąd: Gotham City

Ostrzeżenie: (0%)
-----


W konstruktorze _PDOConnect masz nas stałe
  1. $Dsn = new _DSN('dsn.txt');

powinienes _DSN przesłać w argumencie konstruktora
  1. private function __construct( _DSN $dsn )

i już masz wstrzyknięta zależność

Singelton to zły pomysł, obsługa jednego połączenia też.
Pula połączeń jeśli takowe już istnieje biore z puli.

  1. static $dsnInfo = array();

To też zły pomysł. Zmienne prywatne, gety ,sety tak jak pisze ten alkocholik nademna.

Ten post edytował emp 16.04.2013, 22:05:35


--------------------
Temat zamykam i przenoszę do Bangladeszu.
To jest wiadomość śmierci jeśli ją czytasz to znaczy że pozostało ci 30 sekund życia, więc lepiej zacznij się modlić.
Go to the top of the page
+Quote Post
mstraczkowski
post 17.04.2013, 01:39:23
Post #5





Grupa: Zarejestrowani
Postów: 273
Pomógł: 52
Dołączył: 3.02.2013
Skąd: Przemyśl

Ostrzeżenie: (0%)
-----


Ja dodam, że singletona powinno się unikać, tak samo jak konstrukcji goto lub funkcji eval()
Ten wzorzec nie jest mile widziany podczas tworzenia dobrego obiektowego kodu.

Oczywiście ma on swoje zastosowanie, ale w 99,9% przypadków jest on stosowany w nieprawidłowy sposób.


--------------------
Jeżeli moja wypowiedź Ci pomogła użyj przycisku
Go to the top of the page
+Quote Post
q3trm
post 17.04.2013, 12:29:27
Post #6





Grupa: Zarejestrowani
Postów: 83
Pomógł: 1
Dołączył: 26.02.2013

Ostrzeżenie: (0%)
-----


Cytat(Pilsener @ 16.04.2013, 22:00:25 ) *
1. Jeśli ma to być singleton brakuje prywatnej/chronionej metody __clone
2. Przemyślałbym obsługę wielu baz, bo nawet jeśli tego nie przewidujemy to może taka potrzeba się pojawić. I pytanie, czy musi to być singleton bo przecież nie piszemy skryptu, gdzie w każdym pliku na samym początku DB::getInstance()
3. Współpracę klas oceniam fatalnie - poczytaj o wzorcach współpracy klas, na początek dependency injection, "wstrzykiwanie zależności" - tak to chyba po polsku się nazywa wink.gif
4. Podobnie klasę _DSN - zobacz, jak tego używasz, najpierw tworzysz obiekt by wywołać konstruktor a potem dobierasz się do pól statycznych? smile.gif Podstawą są gettery i settery a nie kombinowanie metodami statycznymi czy magicznymi. A jeśli już to powinieneś pobrać sobie ten cfg jedną metodą statyczną gdzie przekazujesz ten plik jako parametr, bez konieczności tworzenia obiektu.


1. Widziałem sporo wzorców Singleton i w żadnym nie spotkałem zastosowania metody __clone.

2. Nie rozumiem, czemu aktualny kod nie może nawiązywać połączenia z różnymi bazami?. Domyślam się, że chodzi o gotowy kod dla konstruktora klasy PDO i metodę, która będzie go wstrzykiwać w zależności od chęci użycia danej bazy.
Akurat w moim przypadku wystarczy zmodyfikować ręcznie kod w pliku.

3. Poczytam.

4. Racja to było desperackie podejście - bez obycia w programowaniu obiektowym, człowiek ma mętlik w głowie jak przychodzi powiązać klasy ohno-smiley.gif .

Ten post edytował q3trm 17.04.2013, 12:31:10
Go to the top of the page
+Quote Post
viking
post 17.04.2013, 12:58:26
Post #7





Grupa: Zarejestrowani
Postów: 6 365
Pomógł: 1114
Dołączył: 30.08.2006

Ostrzeżenie: (0%)
-----


Ad1. Słabe przykłady miałeś smile.gif
https://github.com/zendframework/zf1/blob/m...y/Zend/Auth.php


--------------------
Go to the top of the page
+Quote Post
q3trm
post 17.04.2013, 15:41:30
Post #8





Grupa: Zarejestrowani
Postów: 83
Pomógł: 1
Dołączył: 26.02.2013

Ostrzeżenie: (0%)
-----


Poprawiłem co nieco semantykę i wiązanie klasy _DSN z _PDOConnect.

Pokażę tylko _PDOConnect

  1. class _PDOConnect
  2. {
  3. private static $instance;
  4. protected $dsnInfo;// przechowuję tablicę danych dsn z klasy _DSN
  5.  
  6. private function __construct()
  7. {}
  8.  
  9. public function getClass_DSN(_DSN $dsn) // delegacja klasy _DSN
  10. {
  11. $this ->dsnInfo = $dsn ->getDsnInfo();
  12. }
  13.  
  14. public function getPDOClass()
  15. {
  16. try
  17. {
  18. $PDO = new PDO (
  19. $this ->dsnInfo['driver'].
  20. $this ->dsnInfo['host'].
  21. $this ->dsnInfo['dbname'].
  22. $this ->dsnInfo['user'].
  23. $this ->dsnInfo['password']
  24. );
  25. }
  26. catch (Exception $e)
  27. {
  28. echo $e ->getMessage();
  29.  
  30. }
  31. }
  32.  
  33. public static function getInstance()
  34. {
  35. if (empty(self::$instance))
  36. {
  37. self::$instance = new _PDOConnect();
  38. }
  39. return self::$instance;
  40. }
  41. }
  42.  
  43. ?>


Tylko teraz, powyższa implementacja klasy Singleton wymaga wywołania trzech metod:
_PDOConnect::getInstance();
getClass_DSN(_DSN $DSN);
getPDOClass();

Tak pomyślałem, że szkoda żeby konstruktor się marnował rolleyes.gif, więc stworzyłem implementację opartą na nawiązywaniu połączenia w konstruktorze - tak jak to było pierwotnie za implementowane.


  1. class _PDOConnect
  2. {
  3. private static $instance;
  4. protected static $dsnInfo; // przechowuję statyczną tablicę danych dsn z klasy _DSN
  5.  
  6. private function __construct()
  7. {
  8. try
  9. {
  10. $PDO = new PDO (
  11. self::$dsnInfo['driver'].
  12. self::$dsnInfo['host'].
  13. self::$dsnInfo['dbname'].
  14. self::$dsnInfo['user'].
  15. self::$dsnInfo['password']
  16. );
  17. }
  18. catch (Exception $e)
  19. {
  20. echo $e ->getMessage();
  21. }
  22. }
  23.  
  24. public static function getClass_DSN(_DSN $dsn) // delegacja klasy _DSN
  25. {
  26. self::$dsnInfo = $dsn ->getDsnInfo();
  27. }
  28.  
  29. public static function getInstance()
  30. {
  31. if (empty(self::$instance))
  32. {
  33. self::$instance = new _PDOConnect();
  34. }
  35. return self::$instance;
  36. }
  37. }
  38.  

_PDOConnect::getClass_DSN(_DSN $DSN);
_PDOConnect::getInstance();

Teraz wystarczą tylko dwie metody i klasa gotowa do użycia. Co o tym sądzicie ma to ręce i nogi?
Go to the top of the page
+Quote Post
Adi32
post 22.04.2013, 14:50:21
Post #9





Grupa: Zarejestrowani
Postów: 348
Pomógł: 26
Dołączył: 8.10.2008
Skąd: Lublin

Ostrzeżenie: (0%)
-----


Stworzyłem takiego dziwaka:

  1. /**
  2.  * Jakieś takie udziwnienie...
  3.  * Odziedzicz po tym a będziesz miał singletona... jak widać
  4.  */
  5. abstract class singleton {
  6.  
  7. final private function __construct() {}
  8. final private function __clone() {}
  9.  
  10. final public static function getInstance() {
  11. return (static::$instance)?self::onlyReturn(): self::loadAndReturn();
  12. }
  13.  
  14. final static private function onlyReturn() {
  15. static::$instance->afterInstance();
  16. return static::$instance;
  17. }
  18.  
  19. final static private function loadAndReturn() {
  20. $called_class = get_called_class();
  21. static::$instance = new $called_class();
  22. static::$instance->afterInstance();
  23. return static::$instance;
  24. }
  25.  
  26. # nadpisz mnie, jestem konstruktorem
  27. protected function afterInstance() {}
  28.  
  29. }


I jest nawet wygodny w użyciu.


--------------------
Wolałem języki z rodziny C ale poszedłem na łatwizne...
Go to the top of the page
+Quote Post

Reply to this topicStart new topic
1 Użytkowników czyta ten temat (1 Gości i 0 Anonimowych użytkowników)
0 Zarejestrowanych:

 



RSS Wersja Lo-Fi Aktualny czas: 26.04.2024 - 21:01