Pomoc - Szukaj - Użytkownicy - Kalendarz
Pełna wersja: Publiczny modyfikator dostępu...
Forum PHP.pl > Forum > PHP > Object-oriented programming
starach
Cześć,

Tak się zastanawiałem ostatnio nad zmianą przyzwyczajeń jeśli chodzi o tą sprawę. Deklarowanie wszystkich składowych jako prywatnych lub chronionych jest ni mniej ni więcej upierdliwe jak pisanie setterów i getterów za pomocą __get() __set(), ale estety trochę mam wrażenie na tym cierpi. Wyskrobałem sobie przed chwilą następujący kod:
  1. class PropertiesInjector
  2. {
  3. /**
  4. * array('id' => array('type' => 'integer'),
  5. * 'name' => array('type' => 'string'),
  6. * 'ip' => array('length' => <int>))
  7. * @var array
  8. */
  9. protected $aPatterns;
  10. /**
  11. * Stores properties and their values
  12. * @var array
  13. */
  14. protected $aProperties;
  15. /**
  16. * Returns class property or throws exception if it does not exists
  17. * @param string $sName
  18. * @return mixed
  19. */
  20. public function __get($sName)
  21. {
  22. $this->_propertyExists($sName);
  23. return isset($this->aProperties[$sName]) ? $this->aProperties[$sName] : null;
  24. }
  25. /**
  26. * Sets class property value or throws an error if value
  27. * is other type than declared or property does not exists
  28. * @param string $sName
  29. * @param mixed $mValue
  30. */
  31. public function __set($sName, $mValue)
  32. {
  33. $this->_propertyExists($sName);
  34.  
  35. if(isset($this->aPatterns[$sName]['type']))
  36. {
  37. $aTypes = $this->aPatterns[$sName]['type'];
  38. $aTypes = \is_array($aTypes) ? $aTypes : array($aTypes);
  39. if(false === \array_search(gettype($mValue), $aTypes))
  40. {
  41. throw new Exception\PropertyValue($sName, $mValue);
  42. }
  43. }
  44. if(isset($this->aPatterns[$sName]['length']))
  45. {
  46. if(\strlen($mValue) > $this->aPatterns[$sName]['length'])
  47. {
  48. throw new Exception\PropertyToLong($sName, $mValue, $this->aPatterns[$sName]['length']);
  49. }
  50. }
  51. $this->aProperties[$sName] = $mValue;
  52. }
  53. /**
  54. * Throws exception if property name is incorrect
  55. * @param string $sName
  56. */
  57. private function _propertyExists($sName)
  58. {
  59. if(!isset($this->aPatterns[$sName])) {
  60. throw new Exception\PropertyName($sName);
  61. }
  62. }
Tak więc się zastanawiam jak to teraz powinno być z tymi modyfikatorami dostępu. Co o tym sądzicie?
batman
Na początek zamieniłbym metodę _propertyExists na __isset.
Poza tym nic nie stoi na przeszkodzie, aby właściwości klasy były publiczne. __set, __get i pozostałem magiczne metody zostały po to wprowadzone, by walidacja typu/zawartości odbywała się w klasie, a nie gdzieś tam w kodzie.
starach
__isset owszem z tym że chciałbym żeby w przypadku nie istniejącej właściwości skrypt płuł mi wyjątkiem czego wydaje mi się nie należy robić przez __isset żeby dać możliwość programiście sprawdzenia czy dana właściwość istnieje.

  1. /**
  2. * Checks whether class property exists or not
  3. * @param string $sName
  4. * @return boolean
  5. */
  6. public function __isset($sName)
  7. {
  8. return isset($this->aPatterns[$sName]);
  9. }
  10. /**
  11. * Throws exception if property name is incorrect
  12. * @param string $sName
  13. */
  14. private function _propertyExists($sName)
  15. {
  16. if(!isset($this->$sName)) {
  17. throw new Exception\PropertyName($sName);
  18. }
  19. }
Czyli nie ma żadnych przeciwskazań. Super.
Crozin
To co obecnie robisz jest kompletnie bez sensu. Jak Ci się nie chce getterów/setterów pisać to wykorzystaj do tego IDE. W Przypadku PHP chyba jedynie NetBeans ma opcję generowania ich.
starach
Nie bierzesz poprawki na to że jakie dobre by to IDE nie było to nie wygeneruje mi setterów z walidacją. Tutaj ona jest najważniejsza. W PHP mogę tylko ustawiać w prototypie nazwy klas, interfejsów i array, ale nic więcej.
Crozin
Jaki jest cel tej walidacji? Nie wygląda mi to na sprawdzanie danych pochodzących od użytkownika (formularze itp.), a sprawdzanie samego programisty jest raczej średnim pomysłem. By to miało sens musiałbyś tworzyć wiele skomplikowanych reguł (w końcu ograniczenie się do samego typu to słaba walidacja). W dodatku w PHP jako języku, który nie przykłada większej uwagi do typu danych mogłoby to powodować niepotrzebne "wymuszenia" typu: doSth((int) $id) (bo ID było stringiem).

btw: Źle używasz wyjątków. To co wyrzucasz (instancję jakiej klasy) powinno być uwarunkowane tym co powoduje dany błąd, a nie skąd on pochodzi. W tym przypadku akurat powinieneś wykorzystać istniejącą już klasę InvalidArgumentException i ewentualnie DomainException.
starach
Ok Crozin argument do mnie dotarł i przestrugałem jedną swoją klasę na coś co wydaje mi się że sugerujesz.

  1. /**
  2.  * @property mixed $id
  3.  * @property string $url
  4.  * @property string $lang
  5.  * @property string $pattern
  6.  * @property boolean $isDefault
  7.  * @property string $controller
  8.  */
  9. class Route
  10. {
  11. private $id;
  12. private $url;
  13. private $lang;
  14. private $pattern;
  15. private $isDefault;
  16. private $controller;
  17. /**
  18. * @var Router\Router
  19. */
  20. static private $Router;
  21. /**
  22. * array containing url parts
  23. * @var array
  24. */
  25. private $aUrl = array();
  26. /**
  27. * Assigns a value to class property using defined setter methods
  28. * @param string $sName
  29. * @param mixed $mValue
  30. */
  31. public function __set($sName, $mValue)
  32. {
  33. if($sName == 'url') {
  34. $this->aUrl = \explode(self::$Router->getPartsSeparator(), $mValue);
  35. } else
  36. if($sName == 'isDefault') {
  37. $sName = 'Default';
  38. }
  39. $sName = 'set'.\ucfirst($sName);
  40. $this->$sName($mValue);
  41. }
  42. /**
  43. * Returns value of class property using getter methods
  44. * @param string $sName
  45. * @return mixed
  46. */
  47. public function __get($sName)
  48. {
  49. if($sName == 'isDefault') { $sName = 'Default'; }
  50. $sName = 'get'.\ucfirst($sName);
  51. return $this->$sName();
  52. }
  53. /**
  54. * Retrieves part of URL
  55. * @param integer $id
  56. * @return mixed Part of URL on success or boolean false on failure
  57. */
  58. public function getUrlPart($id) { return isset($this->aUrl[$id]) ? $this->aUrl[$id] : false; }
  59. /**
  60. * Copies contents of itself to $R
  61. * @param Route $R
  62. */
  63. public function copy(Route $R)
  64. {
  65. foreach(get_object_vars($this) as $sName => $val) { $R->$sName = $val; }
  66. }
  67. // ----------------- //
  68. // SETTERS & GETTERS //
  69. // ----------------- //
  70. public function getId() { return $this->id; }
  71. public function getUrl() { return $this->url; }
  72. public function getLang() { return $this->lang; }
  73. public function getPattern() { return $this->pattern; }
  74. public function getDefault() { return $this->isDefault; }
  75. public function getController() { return $this->controller; }
  76.  
  77. public function setId($id) { $this->id = $id; }
  78. public function setUrl($sUrl) { $this->url = $sUrl; }
  79. public function setLang($sLang) { $this->lang = $sLang; }
  80. public function setPattern($sPattern) { $this->pattern = $sPattern; }
  81. public function setDefault($bDefault) { $this->isDefault = (bool)$bDefault; }
  82. public function setController($sController) { $this->controller = $sController; }
  83. /**
  84. * @param Router\Router $R
  85. */
  86. static public function setRouter(Router\Router $R) { self::$Router = $R; }
  87. }


Niestety twoje tłumeczenie odnośnie wyjątków nie trafia do mnie. Jeśli błędna jest zmienna przekazywana do metody to pluję wyjątkiem informując o tym, a nie o tym co się ewentualnie może przez to stać. Zresztą wymienione przez ciebie InvalidArgumentException jest dokładnie tym samym co u mnie PropertyValue tyle tylko że nie nazywam tego InvalidPropertyValue lub PropertyValueException.
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-2024 Invision Power Services, Inc.