I think it’s good to make setter and getter methods become private ,since we won’t make it publicly accessible (take a look at: http://www.whitewashing.de/2012/08/22/building_an_object_model__no_setters_allowed.html)
But, I see many declare it as public to make it easy to test.
But, being public methods, I think it’ll break the design contract.
Any opinion?
Since the variable that they get/set should be private how do you expect the field to ever be updated if the getter and setter are also private?
The whole point of getters and setters is so the variable itself can be private and you can apply appropriate input and output processing via the getter an setter.
If you don’t want anything outside the object accessing the variable then don’t define the getter and setter at all.
But, the point of the article you linked to is to avoid getters and setters generally. To me, using them shows a simple lack of abstract thinking in forming your model. Although one could argue getters and setters are “KISS”, as Benjamin points out, they also break the open/closed principle in OOP (the O in SOLID). I think Allen Holub explains it best in the article Benjamin linked to.
To see why, consider that there might be 1,000 calls to a getX() method in your program, and each call assumes that the return value is of a particular type. You might store getX()'s return value in a local variable, for example, and that variable type must match the return-value type. If you need to change the way the object is implemented in such a way that the type of X changes, you’re in deep trouble.
Ok. PHP might bend to letting this change still work, but that is not how you should take advantage of PHP’s dynamic typing. And typing is going to get tighter, when performance is needed (see the new strict typing system in PHP7 for example), so don’t plan on this working either.
Scott
I think we can take the advantage that setter and getter encapsulates the logic before storing/retrieving property.
Let say that we want to instantiate a Car object
If the parameter ‘name’ is not a name of car, it will throw exception.
And, if we want to get the car with parameter ‘name’, it checks whether the car is available, and throw exception if it’s not found
Think about the __constructor of Car object like this
``
public function __construct($name)
{
// check if the name is string and available
// set the name of the car
}
``
So, if we don’t want to violate the Open/Closed Principle and want to take advantage of encapsulation, Should we just make the setter and getter private ?
``
public function __construct($name)
{
$this->setName($name);
}
private function setName($name)
{
if(!is_string($name)) { throw new InvalidArgumentException();}
if(in_array($name, $this->whitelist)
{
$this->name = $name;
}
else {throw new Exception('Invalid car name');}
``
Or, is the one below the better solution ?
``
public function __construct($name)
{
if($this->isValidCarName()) { $this->name = $name; }
else {throw new Exception('Invalid Car Name');
}
protected isValidCarName($name)
{
if(is_string($name) && in_array($name, $this->whitelist)) {return TRUE;}
else {return FALSE;}
``
Or, should we just do everything (validating logic+set the car name) in __constructor? If it’s better, why don’t we encapsulate the validating and storing logic?
I see Symfony Validator use Annotation to validate the parameter.
But, I think comments or docblocks shouldn’t take control of the logic.
So, what’s the solution?
It would be better to declare it as private so that the corresponding part of the object’s state isn’t seen by the public. Even I think that it would break the design contract.
I think you are bringing a second problem into the discussion, which means we are talking about two separate concerns. Accessing/ setting private properties and validation.
Firstly, as I understood felgall, your private functions won’t be accessible, which would be defeating the whole point of getters and setters to begin with.
Secondly, a proper validation shouldn’t be the “means” to access your properties.
Validation, if you think about it, is actually some sort of “extra info” about your properties. This “extra info” or “extraneous info” can also be called metadata. So, for (generic) validation rules, using annotation is a valid method to “notate” the valid state of a property. At least, that is how I see it. I agree though, getting too crazy with validation and hooking into annotations isn’t completely optimal either
Think, though, about all the duplicated code your solution would cause. I’d say, you’d want to avoid that.
Scott
[quote]So, if we don’t want to violate the Open/Closed Principle and want to take advantage of encapsulation, Should we just make the setter and getter private?
public function __construct($name) {
$this->setName($name);
}
[/quote]
In fact, you need:
public function __construct($name) {
$this->loadCar($name);
}
Getters and setters will get and set some properties, not load the entire object.
Example:
$myCar = new car('corsica');
$myCar->setColor('red');There are a few schools of thought on getters/setters. However, they’re a necessary evil. The biggest advantage of getters is futureproofing.
Consider the following class:
class Product {
public $price;
}
You have various bits of code that use price e.g.
'The price of the product is' . $procuct->price;
$total = $product->price * $quantity;
If the requirements change and now product prices must include tax at 20% then you have a problem. If you’d used a getter, e.g.
class Product {
private $price;
public function getPrice() {
return $this->price;
}
}
Then the change is a 30 second job:
class Product {
private $price;
public function getPrice() {
return $this->price * 1.2;
}
}
Bertrand Meyer (One of the pioneers of OOP) coined the Uniform Access Principle which states that “Someone using the class should not know whether a value is calculated or stored by the class”.
So that’s why you should use getters and setters. However, a bigger question that’s harder to answer is: When should you use getters and setters?
Are they always necessary? Well, often not as explicitly as you’d think. Consider again this example:
$total = $product->getPrice() * $quantity;
Seems reasonable? Think again. Here encapsulation has been broken. Encapsulation has been defined as
Encapsulation is the packing of data and functions into a single component.
Clearly, in this example the data is being worked on in a function outside the class!
When is this a problem? Well it’s inflexible. The same rule is applied to every product, I’ve worked on e-commerce sites where products have bands (Buy 10 and the unit price goes down by 5%, buy 100 and the unit price goes down by 10%, etc, of course this could be different for different products).
Instead, consider moving the behaviour that uses the data into the class itself:
class Product {
private $price;
public function calculatePrice($quantiy) {
if ($quantity < $this->band1) $unitprice = $this->price;
else if ($quantity > $this->band1) $unitprice = $this->price * $this->discount1;
else if ($quantity > $this->band2) $unitprice = $this->price * $this->discount2;
return $unitprice * $quantity;
}
}
Here the calculation has been moved into the product class (Proper encapsulation) and the need for a simple getter that just returns $this->price has been removed entirely.
So, one could say “get” and “set”, as methods, aren’t really proper behaviors of a Product object (or any other for that matter) ?
Scott
Obviously there are cases where you do just want to extract/set one value but these cases are a lot less frequent than you’d think. Mostly things that are needed for display purposes will need to be accessible, but even then:
public function getName() {
return $this->title. '. ' . $this->firstname . ' '. $this->surname;
}
is better than
public function getFirstName() {
return $this->firstname;
}
public function getSurname() {
return $this->surname;
}
public function getTitle() {
return $this->title;
}
This topic was automatically closed 91 days after the last reply. New replies are no longer allowed.