arborint wrote:I agree, getters/setters are a code smell. They are not evil in themselves. They are useful in many instances. When you seem them in your code you should ask yourself if you are simply hiding procedural steps in multiple calls.
They are a code smell indeed, but they are not evil per se. I was saying that the
fixation on having setters and getters for everything can be an indication of misplaced functionality. Reading back, however, I can imagine that it is easy to misread. Still hard to express myself in English on some occasions, sorry 'bout that.
Your posts did get me to think it through a bit more though, so I have to thank you for it. Yes, getters and setters have their uses. Actually, I had an additional paragraph before I posted my message to this forum stating that although they
could indicate misplaced behaviour, you really can't live without (some) setters/getters. But I figured that might confuse other readers, so I removed that paragraph.
Coming back to this topic however, if you don't mind, I'd like to ask you guys a question. I reckon most people reading this forum are actually separating logic according to the MVC pattern. In my case, an object residing in my model layer is a Domain Object. It _does_ have additional logic, it's not just mapping to a database table. Taking a User for example, a User has a method named resetPassword( $old, $new ) for example. It also contains a User's name, however.
Now, when displaying details of a user, the template will have to echo the user's name. By common standards, it would be bad practise to display it from a public member e.g. $user->name;, so we tend to write a getter for it (or worse yet, we generate it). That's not at all weird, because we don't want the view to know how the object _really_ gets the user's name (it might be lazy loaded, for example).
Public members are definitely out of the question. So, we tend to write a getter for this particular member, to make sure we're not breaking the rules of encapsulation. Problem with writing this particular getter though is that such a getter makes it very easy to misplace functionality that actually belong to the object itself. Some people therefore conclude that all setters and all getters are evil. I strongly disagree with that notion though, since there _are_ legitimate reasons for using setters/getters.
Martin Fowler, as always, has something to say about this too. He states that setters/getters are not evil, but they can be a rather big codesmell. If you get two members from an object to calculate something, or carry out an action with it, that behaviour is probable misplaced and should be placed in the object you're asking the values from. If that behaviour seems to vary, you should encapsulate it to a different object. That's really the essence of OOP (if you ask me

).
In the bliki-article Fowler wrote, there is a quote:
If we're looking for a simple rule of thumb, the one I prefer is one that I first heard from Kent Beck, which is to always beware of cases where some code invokes more than one method on the same object. This occurs with accessors and more reasonable commands. If you ask an object for two bits of data, can you replace this with a single request for the bit of data you're calculating? If you tell an object to do two things, can you replace them with a single command? Of course there are plenty of cases where you can't, but it's always worth asking yourself the question.
In my opinion, mister Fowler is right as usual and the code below should be considered bad encapsulation (and thus a bad design):
Code: Select all
<?php
$user = $this->_usermapper->getUserById( 1 );
$bday = $user->getBirthday( );
$this->view->age = $this->calculateAge( $bday );
?>
If you don't see the horrible mistake in this example: it's the call to $this->calculateAge( $bday ), where it should have been $user->getAge( ). Now.. in this example it's rather obvious where the mistake is, although it can actually be more subtle than this. Had there not been a +getBirthday( ) method, this problem would never have occured.
But then, I can see a lot of situations where the +getBirthday( ) method is actually necessary. To accomplish layering, one would not output the details of a User in the same object that one uses to save this User to the database. That's the Single Responsibility Principle in effect and we've learned to adhere that quite some time ago, since it increases both maintainability, stability, flexibility and as a result, testability.
So, the View in MVC should be able to render proteced members from an object in the model layer. But how then? As we concluded, public members aren't an option, they will absolutely ruin encapsulation. Getters/setters _may_ potentially be abused and you might just miss it. Magic methods are... well.. not my preffered choice for the simple reason that they don't tell you what they do when skimming an object's interface. I'd rather have an object that shows what it does on the interface than having to open the file to see what __call, __get and __set are doing.
Those are the choices we have in PHP: public members, accessor/mutators and magic methods. We don't have properties like C# and Python do. But then... So, we'll have to write getters for each member we wish to output if we want to keep the encapsulation. I think that's the reason people write them. Aren't there other solutions?
One of the solutions is obvious: create a +render( ) method on the User object and pass that a UserRenderer object, which in turn can be used by the View to output the members. It'll probably take 20 minutes at most after you try this method before you start hating for even suggesting this

Yes, indeed, this becomes rather tedious, because you'll have to write a class for each domain object you wish to output times the amount of different output types. Yes, that probably is too tedious when comparing it to the small amount of benifits you get from it.
Another, in my humble opinion more decent and less tedious option, is to have the domain object throw a Data Transfer Object to the View. That is what it's meant for, right? I've not tried this approach yet, but I think I might be on to something here

The point is that the view should not have any access to mutating methods (also known as command) on objects which reside in the model layer. So, in my humble opinion the view should not have access to the User::changePassword( ) action, since that it is not it's concern. The
easiest way I see is to be able to create a Data Transfer Object the template could use.
This could obviously be enforced using an interface which has the method "exportData", and using an abstract class "DomainObject" which defines the behaviour to the call of that defined method. Still, I have my concerns. There aren't many, but a few, methods that are used for displaying values from the Domain Object. In the Netherlands, where I happen to live, names consist not only of a first name and a last name, it can also have a "tussenvoegsel", which most literally means "prefix". To everyone that isn't Dutch, one could compare it to "the". It's a word that exists, but hasn't any importance. By example: using Peter van Stuyvesant, Peter would be the first name, Stuyvesant is the last name, and "van" would be the prefix. Most literaly it means "Peter Of Stuyvesant".
So, when trying to display the name, there is some logic involved. If the name does have a prefix, the prefix plus an additional space should be shown. That logic should be in the model layer, so as to not repeat yourself. Thus, that logic can not be in a Data Transfer Object, since it's not just data, it should actually be in a method called +getName( ). And then what? Now, I tend to just pass on the Domain Object itself. I do wonder however if this is the best way to go, since that would mean I'd pass an object that has more behaviour than just displaying logic (such as logging in). It probably isn't, but I really look forward to reading to what you have to say on this topic.
allspiritseve wrote:Another thing to consider is in a web context, domain objects don't generally have all that much business logic... they are basically data holders that are virtually the same in the database as when displayed. In that sense, they're almost glorified arrays, and getters/setters are the best way to give access to those properties while still leaving room for future business logic.
I've read more of your topics than you would probably imagine (mostly on Sitepoint though), and I tend to agree with you a lot, but in this partical instance, I would like to beg to differ. A web context (assuming you're implying web-based PHP software) does not mean there is less bussiness logic. Worse yet, I'd say the amount of bussiness logic involved in a web application is the same amount as you would find in a desktop application. Zend Framework, Symfony, CodeIgnitor and other frameworks seem to have a tradition in braking the rules of SRP, but the logic is there nevertheless. They just tend to put it in the controller layer, which is a huge mistake in my opinion.
Anyway, do you guys have any thoughts on how to pass an object to the View layer without that object being mutable while still being able to allocate the specific diplaying logic? I'd be really happy to read up on that.
PS: If some of the sentences don't seem to make sense: I'm not only Dutch, but it also happens to be my birthday which means I just had a party, with a lot of wine
