Hello,
I have an object which consists only in setters and getters methods. All the setters are called upon instantiation. Then it’s all about using the getters.
I was wondering if it would acceptable to do that:
$foo = $object->get()->foo;
Instead of:
$foo = $object->getFoo();
Is there any advantage? I somewhow find the syntax cleaner.
How would the internal of the object work to reach the first syntax?
Would there be a third, even shorter, syntax possible?
Generally in the OO world there is an acceptance that getters/setters are bad as they expose too much of the API of objects. They break OO hiding principles and if the scope of the library/application grows large then they make objects much harder to maintain .
Here is a decent article (in the java world) that everyone using in PHP can understand. I does talk a little about Java specifics, but we can garner the rational for trying not to use them: Why getter and setter methods are evil
I hope this gives you some ideas on the subject.
Of note, TobB’s magic method for getters/setters can help when you have a large number of them. The point though, is that you should avoid setter/getters so should their really be a need for this use of the magic methods? You also have to be careful as they make things harder to understand.
To be fair, in PHP most of the criticisms in that article are redundant as php isn’t strongly typed.
Unless I’m mistaken it’s advocating this:
class SomeObjectWithProperties {
protected $foo = 'foo';
protected $bar = 'bar';
public function process(SomeObjectThatNeedsInformation $obj) {
return $obj->formatWithdata($this->foo, $this->bar);
}
}
class SomeObjectThatNeedsInformation {
public function format($foo, $bar) {
return 'Foo: ' . $foo . '<br />Bar:' . $bar;
}
}
$a = new SomeObjectWithProperties;
$b = new SomeObjectThatNeedsInformation;
echo $a->process($b);
Essentially the visitor pattern but I may be mistaken! The problem with this approach, and I’m happy to be proven wrong here, but it seems that you’re moving far too much processing into the first object. For each possible way the information needs to be displayed (for each subset of data) you need a method. It seems very messy to me.
The general idea about reducing this is to provide object that give back values rather than passding data into object.
Yes this can be a bigger deal when looking at strongly typed languages, however the same rings true if you have assessors that when the internals of an object are changed they may need to be updated in hundreds of places in an application.
Let’s look at some of the reasons people use setters/getters in PHP:
Frameworks often require them
Dependency Injection
Instance initialization
Runtime instance state changes/inspection
Data transfer objects
ORM model object
What can happen if getters and setter are overused:
Bloating the interface
Increasing coupling
Decreasing encapsulation
Increased risk of misuse and call sequence errors
If Frameworks required them and you use a framework then it isn’t the end of the world.
People see getter/setter in very many examples so they think that that is how things should be done. But you have a right to argue that it is not.
Constructor based injection is generally considered safer than setter/getters this includes when dependency injection is used in PHP. However you may run up against circular dependencies when doing constructor based dependency injection. In this case you will likely need to use a setter, but luckily this does not happen too often.
Essentially many will argue that public members are bad in OOP design. There is not much of a difference between a public member and a public setter.
The increase of coupling is a bad thing.
In PHP where we have lots of freedom, we can create instances based on configuration or constructor parameter and should be ready to go.
Getters/Setters make it a nightmare to reverse engineer an applications you are asked to take over.
You are right that we can reduce setter/getters in value objects, but we can must also ask ourselves if the data required by objects should really be part of an object and not just passed into it.
Setters and Getter are not always wrong, in some cases limited usage of setters/getters is ok, like:
In cases where you create an instance of a class and still have to allow users to change its state.
Data transfer object. Like when data is stored in an object and is sent over a transfer protocol like JSON, JSONP, or SOAP.
In ORM model objects, as basically it is all the class does is to use setter and getter but makes sense in terms of datablase entries.
I don’t think getters and setter are evil, I just think that we should stop and think what we are doing and are we creating more coupling than required or needlessly reducing information hiding/encapsulation?
Keep in mind that every time you want to refactor a class you will need to locate all the places that implement the getter and setters. This is true every time you add, remove, or change a member you need to check where and how it is set or get?
If you keep the instance variable of an object protect/private, we ensure that other classes don’t depend on them and it gives us the flexibility to chaange a variable’s type or implementation however we want without adversely affecting anything else.
So the idea is for an object to be responsible to compute somethin with its’ own data and then return a result without needing getter/setters.
I can see the negatives of public properties over setters: Public properties are dangerous even in PHP because you could potentially override a dependency with something unexpected. At least setters can enforce integrity by checking what the variable is being set to. That said, the number of times I use setters rather than constructor injection is so minimal this shouldn’t be much of a consideration.
Getters, however, request object state. Any method in a class which returns a value beyond a success/failure condition is essentially a getter. If objects can’t request the state of their dependencies
I can entirely understand the advantages of removing getters and even moreso setters, but I’ve yet to see a viable alternative. The visitor pattern I posted above creates separation of concerns issue and adds complexity.
I am totally open to having my opinion changed on this if someone can provide a sensible answer.
Let’s take a very simplistic case. In a lot of cases accessors are used to transfer data from the model layer to the view layer:
class User {
public $name;
public $email;
public function __construct($name, $email) {
$this->name = $name;
$this->email = $email;
}
}
class UserFormatter {
public function output(User $user) {
return 'Name: ' . $user->name . ' Email: ' . $user->email;
}
}
$user = new User('Tom', 'foo');
$userFormatter = new UserFormatter;
echo $userFromatter->output($user);
Here we’ve enforced separation of concerns. The user object has a state an exposes that state. Which apparently is bad. Whether we use getters or public properties here is irrelevant.
So what’s the solution? One would be to combine both the classes but that’s even worse as it hugely limits flexility and reusability. Using inheritance to fix that causes more problems than it solves. So the visitor pattern solution which retains the separation of concerns and avoids getters looks like this:
interface UserDetails {
public function writeInfo($name, $email);
}
class User {
public $name;
public $email;
public function __construct($name, $email) {
$this->name = $name;
$this->email = $email;
}
public function requestDetails(UserDetails $info) {
$info->writeInfo($this->name, $this->email);
}
}
class UserFormatter implements UserDetails {
protected $output;
public function output(User $user) {
$user->requestInfo($this);
return $this->output;
}
public function writeInfo($name, $email) {
$this->output = 'Name: ' . $name . ' Email' . $email;
}
}
However, is this really an option? This has vastly increased the complexity, maintainability has become a nightmare. I want to add a surname field so now need to modify 3 times as many lines of code.
Beyond that, the User object needs to know what other objects need to know about it. This has removed any idea of separation of concerns. Why should the user object need to have any idea about how it would ever possibly need to be used?
Again, perhaps I am totally misunderstanding but… yes getters and setters cause problems but I can’t see a solution that avoids them and doesn’t cause more problems than it solves. Again, I’m totally open to being proved wrong!
Let’s keep in mind that a common PHP script will take a request, query a database and, format the result in some way and return it to the user. Most of the processing here is transferring that data between the layers. That data needs to be transferred and formatted somehow.
Hopefully you don’t feel we’ve hijacked your thread. I have been putting forth ideas to try to answer your question regarding ‘Are getters/setters bad?’. TomB has presented some valid arguments. So it is up to you to decide if you like the idea of reducing your use of setters/getters or you don’t
TomB,
You are right that using inheritance especially the limited type that PHP allows us to use does not solve this problem. Although in php 5.3 traits allow some of this flexibility where it starts to make sense.
I don’t think you’ve missed anything. I was going to type out some examples of what can be done, but being a lazy programmer I found an article that describes some of the techniques that I use: How to remove getters and setters. Obviously the techniques described can’t be used in ever situation.
I don’t think you should abandon setters and getters just maybe when the opportunities present themselves, consider other ways that don’t use this public interface. If your design is best served using getters/setters then use them, but understand that you weaken the private/protected encapsulation of the objects member/properties that are tied to the getters/setters. This coupling means that it may be harder for you to swap out behaviour in the objects that use them.
Setters and getters in the ‘flat’ sense – aka not verifying what’s calling it with something like debug_backtrace or get_included_files never made any sense to me… it’s an extra far call for ZERO security improvement… at least so far as having a setter present is concerned. If they can set it with the method, what’s the difference?
Now if that method validates input or validates what’s calling it, or sets the value itself without being passed data, THEN setters and getters make sense – but I seem to see it, much like ‘objects for nothing’ being thrown into code any old way lately; like a sick fad programmers discover it, then suddenly like the joker with a hammer sees nothing but nails.
@jsshell: That code example you posted passing $_GET… whiskey tango foxtrot is that?!? Manually passing a superglobal?!? Of course the reverse check logic for no reason and lack of a decent result handler… but that’s just my obsession with passing boolean FALSE instead of null.
To me, you’re describing an (associative) array. It takes a bit more for your data to become an object in the real sense of the word.
<hr>
The difference between a property and a method returning a property is that a method should do some processing first, setter or getter. In your case it looks like you want to store some data and then retrieve it, without much processing. It would make sense if you want to make some under-the-hood redirects or return properties that are not existing, and some other inter-modules compatibilities reasons, otherwise a getter is redundant.
The difference is that you can later expand/change what the getter does if needed. For example if you have a webshop with only one currency and a class representing an item that can be bought with a property price, and later on you want to add another currency you need to change code all over the place. If you had a getter instead, getPrice(), you can rewrite that getter to check which currency in currently in use and return the price using that currency. Small change, done if a few minutes, and a lot less error prone than the alternative.
Which is fine, data processing on a getter – it’s when the setter is, well… not doing any validation or processing that it’s a problem. What I meant was that a lot of people seem to think setters and getters are useful for security, when in a lot of implementations they’re just a waste of code and do nothing in that regard.
I would argue to those that believe that setters and most getters are not security problems or useful for security that they are mistaken. The more you provide any public access to your code there may be attack vectors that can break in. As the public API gets strewn through most code, it is very hard to track and to patch when more significant changes are needed. Thus part of my encouragement to move away from them.
Same reasoning holds though. If you ever wanted to add validation or processing to a property and it’s just a property you need to add a lot of code to your codebase (namely everywhere you set the property), whereas if you already had the setter -albeit completely useless at that point- it’s easy to change the behavior later on by simply changing the setter.
I couldn’t agree more that getters/setters and security have nothing to do with one another, but that doesn’t mean that they can’t serve any other purpose, as stated above
I have to agree with Tom. It’s possible that the concept of getter/setter having a bad name is somewhat more about bad programming than about bad design.
You may call your method fetch() instead of get(), store() instead of set(). The fact is that they are getters and setters. And that’s why objects are instances of classes.
<hr>
This.
Properties are not constants. They can be, but they are not limited to.
Setters change object state. Any method that successfully changes a property is essentially a setter. If objects can’t change state…
…that means I’d have to only use constructors and I’d have to create a new object every time I need a property changed?
[QUOTE=itmitică;5126345]I have to agree with Tom. It’s possible that the concept of getter/setter having a bad name is somewhat more about bad programming than about bad design.
You may call your method fetch() instead of get(), store() instead of set(). The fact is that they are getters and setters. And that’s why objects are instances of classes.
<hr>
This.
Properties are not constants. They can be, but they are not limited to.
Setters change object state. Any method that successfully changes a property is essentially a setter. If objects can’t change state…
…that means I’d have to only use constructors and I’d have to create a new object every time I need a property changed?[/QUOTE] You are correct that getter/setter have been given a bad name and generally frowned upon for all the reasons (Post 7) previously mentioned.
You are also right that properties are not always constants and nor should they be. The fact is, that there are a number of ‘Tell’ methods which include Constructor Passing, Object Composition (of objects that have the most knowledge in regards to a property/process/method(s)), Double Dispatch, Value Objects, and Command Change Set. There is also the idea of designing your objects to not require as much ‘passing around data’ and making your classes more focused (not God Like) and weaving such objects together. All of these are reasonable ways, other than immediately choosing a Setter or Getter.
Again Setter/Getters are not evil, we as programmers just have to think about what we give up, and what difficulties we make for ourselves in doing so. It is not the end of the world to use them. In a small application, it may over-complicate an applications design to not use them. If an application grows, then they can be factored out. However, so many courses, books, and web articles/examples show setters/getters as the way to set and get data in PHP applications. This, in part, is because they are easy examples to demonstrate and use for teaching, but I wonder how may people understand the trouble that one can get into and the alternatives to using setter / getters?
Dogs have state (name, color, breed, hungry) and behavior (barking, fetching, wagging tail).
Say you want to first inspect a dog object rather than alter its behavior state, to establish further interaction course. When you have to inquire about behavior state, getters come to mind: getBarking(), getFetching(), getWaggingTail(). Like one of the sources you’ve mentioned, it’s about messaging.
Are there better alternatives to this messaging? Like you, I believe there are. But getters are easier to work with. It’s like a bad practice becoming standard, a necessary evil.
I posted reasons and ways not to use setters and getters; however as TomB originally said and others have supported, setter / getters cannot or should not always be avoided.
Lets use your Dog class to show a few different things:
class Dog {
protected $name;
protected $date_of_birth;
protected $fur_colour;
protected $collar_type;
protected $collar_collar;
public function __construct ( $name, $date_of_birth, $fur_colour, $collar_type = '', $collar_colour = '') {
$this->name = ucfirst ($name);
$this->date_of_birth = $date_of_birth;
$this->fur_colour = ucfirst ($fur_colour);
$this->changeCollar( $collar_type, $collar_colour );
$this->dog = array();
}
public function describe (){
$this->dog['Name'] = $this->name;
$this->dog['Age'] = $this->age();
$this->dog['Fur Colour'] = $this->isGoingGrey();
$this->dog['Collar'] = $this->getCollar();
$html = '<h3>My Dog</h3><ul>';
foreach($this->dog as $key => $value) {
$html .= "<li>$key: $value</li>";
}
$html .= '</ul>';
echo $html;
}
protected function age() {
$today = date('m/d/Y h:i:s a', time());
return $this->dateDiff($this->date_of_birth, $today);
}
protected function dateDiff($time1, $time2, $precision = 3) {
// If not numeric then convert texts to unix timestamps
if (!is_int($time1)) {
$time1 = strtotime($time1);
}
if (!is_int($time2)) {
$time2 = strtotime($time2);
}
// If time1 is bigger than time2
// Then swap time1 and time2
if ($time1 > $time2) {
$ttime = $time1;
$time1 = $time2;
$time2 = $ttime;
}
// Set up intervals and diffs arrays
$intervals = array('year','month','day','hour','minute','second');
$diffs = array();
// Loop thru all intervals
foreach ($intervals as $interval) {
// Set default diff to 0
$diffs[$interval] = 0;
// Create temp time from time1 and interval
$ttime = strtotime("+1 " . $interval, $time1);
// Loop until temp time is smaller than time2
while ($time2 >= $ttime) {
$time1 = $ttime;
$diffs[$interval]++;
// Create new temp time from time1 and interval
$ttime = strtotime("+1 " . $interval, $time1);
}
}
$count = 0;
$times = array();
// Loop thru all diffs
foreach ($diffs as $interval => $value) {
// Break if we have needed precission
if ($count >= $precision) {
break;
}
// Add value and interval
// if value is bigger than 0
if ($value > 0) {
// Add s if value is not 1
if ($value != 1) {
$interval .= "s";
}
// Add value and interval to times array
$times[] = $value . " " . $interval;
$count++;
}
}
// Return string with times
return implode(", ", $times);
}
protected function isGoingGrey() {
$age = $this->age();
$age = explode(' ' , $age);
$age = (int)$age[0]; //Array index 0 will contain the number of years
switch ( $age ) {
case $age >= 15:
return $this->fur_colour . ' with very grey face, belly, brow and fur around the eyes.';
break;
case $age >= 10:
return $this->fur_colour . ' with grey chin and slightly grey brow.';
break;
case $age >= 7:
return $this->fur_colour . ' plus a little grey on the chin.';
break;
default:
return $this->fur_colour;
}
}
public function changeCollar( $collar_type, $collar_colour ) {
switch ( $collar_type ) { //Some type of validation for this setter, normally test for boundary conditions
case 'leather':
case 'vinyl':
case 'woven':
case 'rope':
$this->collar_type = $collar_type;
break;
default:
$this->collar_type = 'Not a valid collar type';
}
$this->collar_colour = $collar_colour; // Should validate and test for boundary conditions
}
protected function getCollar() {
return "Has a $this->collar_type $this->collar_colour collar";
}
}
$o_Dog = new Dog ('Spot', '11/11/2005', 'black' , 'leather', 'brown');
$o_Dog->describe();
The Age is automatically calculated each day it is run. It only needs to be set in the constructor and as long as the object persists it will display the proper age when $o_Dog->describe(); is called. I’ve seen this type of method written as a setter/getter pair when it is not needed.
The fur colour is automatically calculated by calling the encapsulated age() method. Public methods are not needed. Again this could be written as a setter/getter but the public access to this is not needed, instead it relies on its’ internals.
The name does not change, so can be set by the constructor
New collars tend to be purchased throughout a dog’s life, so there is a setter changeCollar method. This is a situation where a setter is a good option.
Obviously more error checking should be done than has been shown, but this is a fairly simple example.
I think this is not a bad (but also not great) example of when a setter is needed for a state change but is avoided in others.
In that dog example you have a describe function which does a lot of display logic. Surely separation of concerns would require this to be done elsewhere. The problem it creates is limited flexibility. The details on the dog can only ever be displayed in one way. Maybe sometimes I only want to show the name and age? Do I need to add a new method to the dog class for that? It’s very messy.
Essentially it’s push vs pull. Something (e.g. display logic) needs to know the details about the dog. A push method (the dog passing the values into the display logic) means the dog class must know what information is needed for display, breaking separation of concerns.
Using this push method (e.g. pass something into the describe function which contains the display logic) if I add a new property, let’s say weight, to display this, the method which pushes the data into the display logic needs to be updated.
Using a pull method: public properties (or getters) the display logic can request what it needs from the Dog object when it needs it. It’s a single edit to add/remove a field because you’re avoiding a step.
The added complexity of using double dispatch or the lack of flexibility and loss of separation of concerns introduced by your describe function, to me, are far worse than the issues which arise from getters/public properties in most cases.
That said, I think your example is good because you’ve put as much logic as possible in the dog class itself. You’ve avoided setters which don’t do anything and kept everything reusable. But that’s avoiding setters as much as getters. A lot of your methods are essentially getters. Your age() function is a getter, for instance.
