Pomoc - Szukaj - Użytkownicy - Kalendarz
Pełna wersja: Moja pierwsza klasa - rejestr
Forum PHP.pl > Inne > Oceny
q.michal
Czesc wszystkim,

Poznaje PHP i w ramach nauki napisalem swoja 1 klase do obslugi rejestru. Bede wdzieczny za wszelka krytyke.

  1. final class Registry {
  2. private $name;
  3. private $registry;
  4.  
  5. /**
  6.   * Disables object cloning
  7.   *
  8.   * @return void does not return any value
  9.   */
  10. private function __clone() {
  11. throw new Exception('Cloning Registry object is not allowed!');
  12. }
  13.  
  14. /**
  15.   * Class constructor
  16.   *
  17.   * @param string instance name
  18.   * @return void does not return any value
  19.   */
  20. private function __construct($name) {
  21. $this->name = $name;
  22. $this->registry = array();
  23. }
  24.  
  25. /**
  26.   * Adds an element to the registry
  27.   *
  28.   * @param string location in the registry, in which to store the value
  29.   * @param mixed value to be stored in the registry
  30.   * @param boolean specifies whether the existing element should be overwritten or not
  31.   * @return boolean TRUE on success, FALSE otherwise
  32.   */
  33. public function addValue($index, $value, $overwrite = false) {
  34. if($this->isRegistered($index) && !$overwrite) {
  35. return false;
  36. }
  37. $this->registry[$index] = $value;
  38. return true;
  39. }
  40.  
  41. /**
  42.   * Adds multiple elements to the registry
  43.   *
  44.   * @param array associative array containing elements to add to the registry
  45.   * @param boolean specified whether the existing elements should be overwritten or not
  46.   * @return boolean TRUE on success, FALSE otherwise
  47.   */
  48. public function addValues($values, $overwrite = false) {
  49. if(!is_array($values)) {
  50. return false;
  51. }
  52. foreach($values as $index => $value) {
  53. if(!$this->addValue($index, $value, $overwrite)) {
  54. return false;
  55. }
  56. }
  57. return true;
  58. }
  59.  
  60. /**
  61.   * Flushes (removes all elements from) the registry
  62.   *
  63.   * @return void does not return any value
  64.   */
  65. public function flushRegistry() {
  66. $this->registry = array();
  67. }
  68.  
  69. /**
  70.   * Returns the global Registry object, creating it only if it does not exist already
  71.   *
  72.   * @param string instance name
  73.   * @return object the QRegistry object
  74.   */
  75. public static function getInstance($name = 'main') {
  76. static $instance;
  77. if(!isset($instance[$name])) {
  78. $instance[$name] = new Registry();
  79. }
  80. return $instance[$name];
  81. }
  82.  
  83. /**
  84.   * Returns all elements stored in registry
  85.   *
  86.   * @return array associative array containing all registered values
  87.   */
  88. public function getRegistry() {
  89. return $this->registry;
  90. }
  91.  
  92. /**
  93.   * Returns the Registry instance name
  94.   *
  95.   * @return string instance name
  96.   */
  97. public function getRegistryName() {
  98. return $this->name;
  99. }
  100.  
  101. /**
  102.   * Retrieves a value from the registry
  103.   *
  104.   * @param string location in the registry, in which the value is stored
  105.   * @param mixed the default value to return when element is not found
  106.   * @return mixed the value of requested item
  107.   */
  108. public function getValue($index, $default = NULL) {
  109. if(!$this->isRegistered($index)) {
  110. return $default;
  111. }
  112. return $this->registry[$index];
  113. }
  114.  
  115. /**
  116.   * Retrieves multiple values from the registry
  117.   *
  118.   * @param array array containing the names of the requested elements
  119.   * @param mixed the default value to return when element is not found
  120.   * @return array associative array containing values of the requested items
  121.   */
  122. public function getValues($indexes, $default = NULL) {
  123. $results = array();
  124. if(!is_array($indexes)) {
  125. return $results;
  126. }
  127. foreach($indexes as $index) {
  128. $results[$index] = $this->getValue($index, $default);
  129. }
  130. return $results;
  131. }
  132.  
  133. /**
  134.   * Checks if container contains an item with the specified index
  135.   *
  136.   * @param string location in the registry
  137.   * @return boolean TRUE if index is a named value in the registry, FALSE otherwise
  138.   */
  139. public function isRegistered($index) {
  140. return array_key_exists($index, $this->registry);
  141. }
  142.  
  143. /**
  144.   * Removes given registered element
  145.   *
  146.   * @param string location in the registry
  147.   * @return boolean TRUE on success, FALSE otherwise
  148.   */
  149. public function removeValue($index) {
  150. if(!$this->isRegistered($index)) {
  151. return false;
  152. }
  153. unset($this->registry[$index]);
  154. return true;
  155. }
  156.  
  157. /**
  158.   * Removes multiple given registered elements
  159.   *
  160.   * @param array array containing the names of the requested elements
  161.   * @return boolean TRUE on success, FALSE otherwise
  162.   */
  163. public function removeValues($indexes) {
  164. if(!is_array($indexes)) {
  165. return false;
  166. }
  167. foreach($indexes as $index) {
  168. if(!$this->removeValue($index)) {
  169. return false;
  170. }
  171. }
  172. return true;
  173. }
  174.  
  175. }
Pyton_000
- Konstruktor bez sensu.
- getInstance referencja ble....

-
Kod
addValue(0, null);
isRegistered(0) -> false

q.michal
Cytat(Pyton_000 @ 14.02.2016, 13:13:11 ) *
- Konstruktor bez sensu.

Mozesz nieco rozwinac?

Cytat(Pyton_000 @ 14.02.2016, 13:13:11 ) *
- getInstance referencja ble...

Masz racje, referencje mozna wywalic.

Cytat(Pyton_000 @ 14.02.2016, 13:13:11 ) *
-
Kod
addValue(0, null);
isRegistered(0) -> false

Racja, uzyje array_key_exists();
com
Cytat
Cytat(Pyton_000 @ 14.02.2016, 13:13:11 ) *
- Konstruktor bez sensu.
Mozesz nieco rozwinac?


no bo co on daje, nic, równie dobrze mogło by być tak
  1. final class Registry {
  2. private $registry = array();
  3.  
  4. /**
  5.   * Class constructor
  6.   */
  7. public function __construct() {}
  8.  
  9. }


poza tym to singleton wiec nie robi się jawnego konstruktora
q.michal
To wszystko prawda, jednak nadal nie rozumiem co jest zlego w tym konstruktorze i dlaczego nie powinno sie go stosowac w przypadku singletonu?
Efekt jest ten sam, kiedy chodzi o dzialanie aplikacji, dlatego licze, ze ktos wyjasni mi ta roznice wink.gif
com
bo w singletonie chodzi przecież o to żeby była 1 instancja tego samego obiektu, wiec jak skonstruujesz 2 to już nie będzie to singleton
https://pl.wikisource.org/wiki/Singleton_(w...towy)/kod#PHP_5
q.michal
Cytat(com @ 14.02.2016, 14:17:51 ) *
bo w singletonie chodzi przecież o to żeby była 1 instancja tego samego obiektu, wiec jak skonstruujesz 2 to już nie będzie to singleton
https://pl.wikisource.org/wiki/Singleton_(w...towy)/kod#PHP_5


Byc moze czegos nie rozumiem, ale w dalszym ciagu jezeli uzyjesz operatora new, zamiast metody getInstance to dostaniesz 2 rozne obiekty, bo to ona dba o to, aby byl to singleton.
Poza tym metoda getInstance pozwala na utworzenie wielu rejestrow. Na czym ma wiec polegac rzekoma roznica? smile.gif
com
no wiec nie rozumiesz singletona, po to stworzyłeś metodę getInstance(...) , żeby wywołać go tak
Cytat
$singleton = Registry::getInstance();
operatora new nie używasz, dlatego konstruktor musi być prywatny, żeby go przez przypadek nie wywołać, a jak spróbujesz to wywali bład.

http://ideone.com/hJodgn

Skoro tworzysz wiele singletonów to poco Ci pattern singleton, skoro łamiesz jego podstawowa zasadę i traci on wgl sens.
q.michal
Inaczej.

Zgadzam sie, ze nie da sie uzyc operatora new jezeli konstruktor jest prywatny. Niemniej jednak w dalszym ciagu nie widze roznicy pomiedzy inicjalizacja zmiennej podczas jej definicji, a w konstruktorze?
Znalazlem natomiast dzieki Tobie blad w kodzie - konstruktor powinien byc prywatny, a u mnie jest publiczny.
com
nie ma różnicy, ale w konstruktorze wypełnia się ją wartościami domyślnymi, poza tym konstruktora nie wywołujesz, wiec to się nigdy tam nie ustawi. (jawnie za pomocą operatora new, zbyt duży skrót myślowy) Poczytaj jak działa singleton, bo kompletnie go nie rozumiesz. I w takiej implementacji jak zrobiłeś jego użycie nie miało sensu.
q.michal
Jezeli konstruktora nie wywoluje, to:

  1. $reg = Registry::getInstance();
  2. var_dump($reg->getRegistry();


nie powinno chyba zwrocic ponizszego wyniku?

Kod
array(0) {
}


Oczywiscie kontruktor jest prywatny, tak jak w 1 poscie.
Chyba, ze naprawde czegos nie rozumiem?
com
nie wywołujesz go jawnie, wiec ustawianie w nim czegoś jaki ma sens? konstruktor wykorzystuje się do tworzenia obiektu i ty go tworzysz tu:
  1. $instance[$name] = new Registry();


Wiec on się tam wywoła, tylko ustawianie w nim pustej tablicy na elemencie nie ma sensu, btw wgl brakuje Ci pola $instance.

Z singletona zrobiłeś sobie zwykła klasę tylko na około nie wiedzieć poco. bo mogę zrobić tak:
  1. $rega = Registry::getInstance('a');
  2. $regb = Registry::getInstance('b');
  3. var_dump($rega === $regb); // bool false

Jeśli masz taki przypadek
  1. class Example
  2. {
  3. private $field;
  4.  
  5. public function __construct($field)
  6. {
  7. $this->field = $field;
  8. }
  9. }


Wtedy to ma sens, ale jak masz prywatny konstruktor to nic to nie daje
Fred1485
Jeśli już o singletonie mówiąc to nie powinno się jeszcze dodać prywatnej metody __clone?
com
Fred1485 owszem

q.michal poczytaj o registry pattern i napisz go jeszcze raz tu masz przykład http://www.phpbar.de/w/Registry
q.michal
Ale chodzi tylko o ten kontrowersyjny konstruktor? Czy ogolnie uwazasz, ze wzorzec zostal zle zaimplementowany?
com
no poco tam wepchałeś singleton, który stracił sens singletona.

Wyrzuć ten konstruktor, ten przykład który Ci dałem miał zobrazować kiedy się go używa a konstruktor robi się prywatny własnie po to żeby go nie używać wiec też go nie używaj. Uparłeś się żeby go mieć i nauczysz się złych praktyk w ten sposób.

  1. $rega = Registry::getInstance('a');
  2. $regb = Registry::getInstance('b');
  3. var_dump($rega === $regb); // bool false
  4. $rega->addValue(0,'test');
  5. var_dump($rega->getRegistry(), $regb->getRegistry());
  6. /*
  7. array(1) {
  8.   [0]=>
  9.   string(4) "test"
  10. }
  11. array(0) {
  12. }*/


Robiąc mały test, dla singletona spodziewasz się że $rega === $regb oraz że $regb->getRegistry() i $rega->getRegistry() zwróci
array(1) {
[0]=>
string(4) "test"
}, bo na tym polega antywzorzec singleton, a u Ciebie on nie ma sensu bo to są dwa rożne obiekty, ale zamiast jawnego konstruktora maja metodę Instance wink.gif

tak działa singleton:
http://ideone.com/RUd4EU
q.michal
Ale rozumiesz, ze getInstance zwraca zawsze ta sama instancje rejestru o tej samej nazwie? Dzieki temu mozesz stworzyc kilka rejestrow. Jezeli nie przekazesz nazwy - uzyje domyslnej.
Jezeli zmodyfikujesz powyzszy kod do:

  1. $rega = Registry::getInstance();
  2. $regb = Registry::getInstance();


to to co piszesz bedzie prawda.
com
Ale na tym polega singleton, jeśli używasz wzorca, to go używaj zgodnie z definicją.

  1. $rega = new Registry('a');
  2. $regb = new Registry('b');


Da Ci dokładnie to samo jak użyjesz depedency injection.

Jak chcesz mieć fabrykę to używasz factory pattern a nie robisz z singletona...
Fred1485
Może jakbyś zrobił abstrakcyjną klasę Registry, a potem poszczególne klasy dziedziczące po niej i każda byłaby singletonem, chyba że nazwy mają być zmienne. Tak na moją głowę tongue.gif
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-2018 Invision Power Services, Inc.