Moja pierwsza klasa - rejestr Opcje

[q.michal]

Czesc wszystkim,

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

[PHP] pobierz, plaintext 
final class Registry {
        private $name;
        private $registry;
 
        /**
         * Disables object cloning
         *
         * @return      void            does not return any value
         */
        private function __clone() {
                throw new Exception('Cloning Registry object is not allowed!');
        }
 
        /**
         * Class constructor
         *
         * @param       string          instance name
         * @return      void            does not return any value
         */
        private function __construct($name) {
                $this->name = $name;
                $this->registry = array();
        }
 
        /**
         * Adds an element to the registry
         *
         * @param       string          location in the registry, in which to store the value
         * @param       mixed           value to be stored in the registry
         * @param       boolean         specifies whether the existing element should be overwritten or not
         * @return      boolean         TRUE on success, FALSE otherwise
         */
        public function addValue($index, $value, $overwrite = false) {
                if($this->isRegistered($index) && !$overwrite) {
                        return false;
                }
                $this->registry[$index] = $value;
                return true;
        }
 
        /**
         * Adds multiple elements to the registry
         *
         * @param       array           associative array containing elements to add to the registry
         * @param       boolean         specified whether the existing elements should be overwritten or not
         * @return      boolean         TRUE on success, FALSE otherwise
         */
        public function addValues($values, $overwrite = false) {
                if(!is_array($values)) {
                        return false;
                }
                foreach($values as $index => $value) {
                        if(!$this->addValue($index, $value, $overwrite)) {
                                return false;
                        }
                }
                return true;
        }
 
        /**
         * Flushes (removes all elements from) the registry
         *
         * @return      void            does not return any value
         */
        public function flushRegistry() {
                $this->registry = array();
        }
 
        /**
         * Returns the global Registry object, creating it only if it does not exist already
         *
         * @param       string          instance name
         * @return      object          the QRegistry object
         */
        public static function getInstance($name = 'main') {
                static $instance;
                if(!isset($instance[$name])) {
                        $instance[$name] = new Registry();
                }
                return $instance[$name];
        }
 
        /**
         * Returns all elements stored in registry
         *
         * @return      array           associative array containing all registered values
         */
        public function getRegistry() {
                return $this->registry;
        }
 
        /**
         * Returns the Registry instance name
         *
         * @return      string          instance name
         */
        public function getRegistryName() {
                return $this->name;
        }
 
        /**
         * Retrieves a value from the registry
         *
         * @param       string          location in the registry, in which the value is stored
         * @param       mixed           the default value to return when element is not found
         * @return      mixed           the value of requested item
         */
        public function getValue($index, $default = NULL) {
                if(!$this->isRegistered($index)) {
                        return $default;
                }
                return $this->registry[$index];
        }
 
        /**
         * Retrieves multiple values from the registry
         *
         * @param       array           array containing the names of the requested elements
         * @param       mixed           the default value to return when element is not found
         * @return      array           associative array containing values of the requested items
         */
        public function getValues($indexes, $default = NULL) {
                $results = array();
                if(!is_array($indexes)) {
                        return $results;
                }
                foreach($indexes as $index) {
                        $results[$index] = $this->getValue($index, $default);
                }
                return $results;
        }
 
        /**
         * Checks if container contains an item with the specified index
         *
         * @param       string          location in the registry
         * @return      boolean         TRUE if index is a named value in the registry, FALSE otherwise
         */
        public function isRegistered($index) {
                return array_key_exists($index, $this->registry);
        }
 
        /**
         * Removes given registered element
         *
         * @param       string          location in the registry
         * @return      boolean         TRUE on success, FALSE otherwise
         */
        public function removeValue($index) {
                if(!$this->isRegistered($index)) {
                        return false;
                }
                unset($this->registry[$index]);
                return true;
        }
 
        /**
         * Removes multiple given registered elements
         *
         * @param       array           array containing the names of the requested elements
         * @return      boolean         TRUE on success, FALSE otherwise
         */
        public function removeValues($indexes) {
                if(!is_array($indexes)) {
                        return false;
                }
                foreach($indexes as $index) {
                        if(!$this->removeValue($index)) {
                                return false;
                        }
                }
                return true;
        }
 
}
[PHP] pobierz, plaintext 

Ten post edytował q.michal 14.02.2016, 18:06:41

[Pyton_000]

- Konstruktor bez sensu.
- getInstance referencja ble....

-

Kod

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

[q.michal]

- Konstruktor bez sensu.

Mozesz nieco rozwinac?

- getInstance referencja ble...

Masz racje, referencje mozna wywalic.

-

Kod

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

Racja, uzyje array_key_exists();

Ten post edytował q.michal 14.02.2016, 13:38:58

[com]

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

[PHP] pobierz, plaintext 
final class Registry {
        private $registry = array();
 
        /**
         * Class constructor
         */
        public function __construct() {}
 
}
[PHP] pobierz, plaintext 

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

[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]

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?

[com]

no wiec nie rozumiesz singletona, po to stworzyłeś metodę getInstance(...) , żeby wywołać go tak

$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.

Ten post edytował com 14.02.2016, 14:46:24

[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.

Ten post edytował q.michal 14.02.2016, 14:57:30

[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.

Ten post edytował com 14.02.2016, 15:19:52

[q.michal]

Jezeli konstruktora nie wywoluje, to:

[PHP] pobierz, plaintext 
$reg = Registry::getInstance();
var_dump($reg->getRegistry();
[PHP] pobierz, plaintext 

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:

[PHP] pobierz, plaintext 
$instance[$name] = new Registry();
[PHP] pobierz, plaintext 

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:

[PHP] pobierz, plaintext 
$rega = Registry::getInstance('a');
$regb = Registry::getInstance('b');
var_dump($rega === $regb); // bool false
[PHP] pobierz, plaintext 

Jeśli masz taki przypadek

[PHP] pobierz, plaintext 
class Example 
{
   private $field;
 
   public function __construct($field)
   {
       $this->field = $field;
   }
}
[PHP] pobierz, plaintext 

Wtedy to ma sens, ale jak masz prywatny konstruktor to nic to nie daje

Ten post edytował com 14.02.2016, 15:25:37

[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.

[PHP] pobierz, plaintext 
$rega = Registry::getInstance('a');
$regb = Registry::getInstance('b');
var_dump($rega === $regb); // bool false
$rega->addValue(0,'test');
var_dump($rega->getRegistry(), $regb->getRegistry()); 
/* 
array(1) {
  [0]=>
  string(4) "test"
}
array(0) {
}*/
[PHP] pobierz, plaintext 

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

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:

[PHP] pobierz, plaintext 
$rega = Registry::getInstance();
$regb = Registry::getInstance();
[PHP] pobierz, plaintext 

to to co piszesz bedzie prawda.

[com]

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

[PHP] pobierz, plaintext 
$rega = new Registry('a');
$regb = new Registry('b'); 
[PHP] pobierz, plaintext 

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...

Ten post edytował com 14.02.2016, 19:26:35

[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ę