Publiczny modyfikator dostępu..., ...a może jednak korzystać? Opcje

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

[PHP] pobierz, plaintext 
class PropertiesInjector
{
	/**
	 * array('id' => array('type' => 'integer'),
	 *       'name' => array('type' => 'string'),
	 *			 'ip' => array('length' => <int>))
	 * @var array
	 */
	protected $aPatterns;
	/**
	 * Stores properties and their values
	 * @var array
	 */
	protected $aProperties;
	/**
	 * Returns class property or throws exception if it does not exists
	 * @param string $sName
	 * @return mixed
	 */
	public function  __get($sName)
	{
		$this->_propertyExists($sName);
		return isset($this->aProperties[$sName]) ? $this->aProperties[$sName] : null;
	}
	/**
	 * Sets class property value or throws an error if value
	 * is other type than declared or property does not exists
	 * @param string $sName
	 * @param mixed $mValue
	 */
	public function __set($sName, $mValue)
	{
		$this->_propertyExists($sName);
 
		if(isset($this->aPatterns[$sName]['type']))
		{
			$aTypes = $this->aPatterns[$sName]['type'];
			$aTypes = \is_array($aTypes) ? $aTypes : array($aTypes);
			if(false === \array_search(gettype($mValue), $aTypes))
			{
				throw new Exception\PropertyValue($sName, $mValue);
			}
		}
		if(isset($this->aPatterns[$sName]['length']))
		{
			if(\strlen($mValue) > $this->aPatterns[$sName]['length'])
			{
				throw new Exception\PropertyToLong($sName, $mValue, $this->aPatterns[$sName]['length']);
			}
		}
		$this->aProperties[$sName] = $mValue;
	}
	/**
	 * Throws exception if property name is incorrect
	 * @param string $sName
	 */
	private function _propertyExists($sName)
	{
		if(!isset($this->aPatterns[$sName])) {
			throw new Exception\PropertyName($sName);
		}
	}
[PHP] pobierz, plaintext 

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.

[PHP] pobierz, plaintext 
/**
	 * Checks whether class property exists or not
	 * @param string $sName
	 * @return boolean
	 */
	public function __isset($sName)
	{
		return isset($this->aPatterns[$sName]);
	}
	/**
	 * Throws exception if property name is incorrect
	 * @param string $sName
	 */
	private function _propertyExists($sName)
	{
		if(!isset($this->$sName)) {
			throw new Exception\PropertyName($sName);
		}
	}
[PHP] pobierz, plaintext 

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.

[PHP] pobierz, plaintext 
/**
 * @property mixed $id
 * @property string $url
 * @property string $lang
 * @property string $pattern
 * @property boolean $isDefault
 * @property string $controller
 */
class Route
{
	private $id;
	private $url;
	private $lang;
	private $pattern;
	private $isDefault;
	private $controller;
	/**
	 * @var Router\Router
	 */
	static private $Router;
	/**
	 * array containing url parts
	 * @var array
	 */
	private $aUrl = array();
	/**
	 * Assigns a value to class property using defined setter methods
	 * @param string $sName
	 * @param mixed $mValue
	 */
	public function __set($sName, $mValue)
	{
		if($sName == 'url') {
			$this->aUrl = \explode(self::$Router->getPartsSeparator(), $mValue);
		} else
		if($sName == 'isDefault') {
			$sName = 'Default';
		}
		$sName = 'set'.\ucfirst($sName);
		$this->$sName($mValue);
	}
	/**
	 * Returns value of class property using getter methods
	 * @param string $sName
	 * @return mixed
	 */
	public function __get($sName)
	{
		if($sName == 'isDefault') { $sName = 'Default'; }
		$sName = 'get'.\ucfirst($sName);
		return $this->$sName();
	}
	/**
	 * Retrieves part of URL
	 * @param integer $id
	 * @return mixed Part of URL on success or boolean false on failure
	 */
	public function getUrlPart($id) { return isset($this->aUrl[$id]) ? $this->aUrl[$id] : false; }
	/**
	 * Copies contents of itself to $R
	 * @param Route $R
	 */
	public function copy(Route $R)
	{
		foreach(get_object_vars($this) as $sName => $val) { $R->$sName = $val; }
	}
	// ----------------- //
	// SETTERS & GETTERS //
	// ----------------- //
	public function getId() { return $this->id; }
	public function getUrl() { return $this->url; }
	public function getLang() { return $this->lang; }
	public function getPattern() { return $this->pattern; }
	public function getDefault() { return $this->isDefault; }
	public function getController() { return $this->controller; }
 
	public function setId($id) { $this->id = $id; }
	public function setUrl($sUrl) { $this->url = $sUrl; }
	public function setLang($sLang) { $this->lang = $sLang; }
	public function setPattern($sPattern) { $this->pattern = $sPattern; }
	public function setDefault($bDefault) { $this->isDefault = (bool)$bDefault; }
	public function setController($sController) { $this->controller = $sController; }
	/**
	 * @param Router\Router $R
	 */
	static public function setRouter(Router\Router $R) { self::$Router = $R; }
}
[PHP] pobierz, plaintext 

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.