Beefy Boxes and Bandwidth Generously Provided by pair Networks
Keep It Simple, Stupid
 
PerlMonks  

Why get() and set() accessor methods are evil

by synistar (Pilgrim)
on Nov 25, 2003 at 16:46 UTC ( #309952=perlmeditation: print w/ replies, xml ) Need Help??

Here on PM get() and set() methods seem to be pretty commonly advocated for. I think there are some pretty good reasons why their overuse should be avoided in object oriented designs. The primary problem with them is that they can defeat the data encapsulation principle of OO design.

Here is a simple example:

package OOtest; sub new { my $invoker = shift; my $class = ref($invoker) || $invoker; my $self = {}; bless ($self, $class); $self->{list} = []; return $self; } sub get_list { my $self = shift; return $self->{list}; }

Later on we use the accessor:

my $obj = OOtest->new(); foreach my $tmp ( @{ $obj->get_list() } ) { # process list }

If we later decide to change the implementation of our list to a hash our loop will also process the hash keys (this would most likely not be what we want). We have to change code outside our object to conform with the new internal data structure of the object. The accessor method is breaking our data encapsulation. set() methods are especially vulnerable to this.

Accessor methods should be used cautiously and only when unavoidable. Instead of using accessor methods write methods that provide "behaviours" for your objects. This moves more code into your objects but provides much better data encapsulation. Then making changes to your object's should only require you to change code inside you object.

UPDATE: I agree this example is pretty flimsy. But I wanted an example that didn't require to much explanation. A better example might be an accessor that returns a hash generated by an internal sub. Bugs or changes in the hash generating sub could affect code outside of the object. The point I was trying to emphasize is the fact that a connection exists between code outside of your object and the internal data representation. The accessor methods are what create this undesirable dependancy.
I also corrected a typo in the example.

UPDATE: After looking at this meditation again and thinking about some of the great comments, I agree with some of the other monks that the tone this node takes is a little too harsh. I primarily wanted to give a warning for people new to OO. The "make an object by just giving it a bunch of public accessors" approach seems to be a common mistake. However, there are good reasons for using them sometimes. And when you do use them realize that they are a major portion of your object's interface.

A lot more has been written about this elsewhere:
"Why getter and setter methods are evil".

Another way out of this hole is to return whole objects:
"Return New Objects From Accessor Methods"

Both links are Java articles but the design issues are universal.

Comment on Why get() and set() accessor methods are evil
Select or Download Code
•Re: Why get() and set() accessor methods are evil
by merlyn (Sage) on Nov 25, 2003 at 17:02 UTC
    An object should have methods reflecting its external behavior. "Set your X value to 24" is rarely a nice reusable, maintainable external behavior.

    People writing these kinds of objects generally haven't quite gotten in to "object thinking", and instead treat the object as a "smart record". Methods should instead be derived by understanding how the class might be used (including subclassing).

    I think the Perl6 object approach where all variables are private to the class (and not even directly available to the subclasses) will make for some interesting designs, especially when thinking about reuse.

    -- Randal L. Schwartz, Perl hacker
    Be sure to read my standard disclaimer if this is a reply.

      I don't see how making attributes private to the class instead tucking them away into a reference makes people not treat objects as glorified structs using accessors to get to the internals.

      For me, objects are state-keepers. Methods may cause the object to change state, or they may be used to interrogate the object about its state (or both). How to object keeps its state however, is something only known the the class(es) of which the object is an instance.

      Abigail

      Given that Larry's mandated that the Perl 6 compiler be capable of autogenerating lvalue accessor methods for attributes, I don't think you'll find that perl 6 will magically make things any better--you'll just end up with a lot of public attribute accessors.

      Granted, a.b = 12; is much nicer looking than a.set_b(12); or a.set_b = 12

        ...but not as nice as a->set_b(12). :-)
        ...I know, I know arrow notaion is going away in Perl6. :-(

        Granted, a.b = 12; is much nicer looking than a.set_b(12); or a.set_b = 12
        (Other post: If it's supposed to be an assignment it should look like an assignment, and setting a property or attribute is an assignment no matter how it's mediated under the hood.)

        FYI, in case you didn't know about it already, Attribute::Property makes a->b = 12 work in Perl 5. From its synopsis:

        my $object = SomeClass->new(digits => '123'); $object->nondigits = "abc"; $object->digits = "123"; $object->anyvalue = "abc123\n"; $object->anyvalue('archaic style still works'); my $john = Person->new(name => 'John Doe', age => 19); $john->age++; printf "%s is now %d years old", $john->name, $john->age;

        When modifying a value using an accessor method (which we dubbed 'archaic style'), things get ugly:

        Old: $john->age($john->age + 1); $john->name->(do { (my $n = $john->name) =~ s/Doe/Smith/; $n }); New: $john->age++; $john->name =~ s/Doe/Smith/;
        Having these things1 built-in in Perl 6 will make me happy.

        Juerd # { site => 'juerd.nl', plp_site => 'plp.juerd.nl', do_not_use => 'spamtrap' }

        1 The things that are called 'properties' in Perl 5 jargon2, but 'attributes' in Perl 6 jargon. Perl 5's 'attributes' are 'traits' in Perl 6. (IIRC)
        2 But not officially. They're often called 'attributes', but we should discourage that, because 'attributes' are already something else. Attributes are relatively new, though, so we can't blame writers of older documentation.
Re: Why get() and set() accessor methods are evil
by chromatic (Archbishop) on Nov 25, 2003 at 17:11 UTC

    While making all of your accessors and mutators public is usually a bad idea, your example is just silly. If you change a public interface, of course things will break!

Re: Why get() and set() accessor methods are evil
by eric256 (Parson) on Nov 25, 2003 at 17:19 UTC

    Shouldn't the example code be...

    my $obj = OOtest->new(); foreach my $tmp ( @{ $obj->get_array() } ) { # process list }

    update: guess so since you changed it (well you changed them both to get_list) :)


    ___________
    Eric Hodges
Re: Why get() and set() accessor methods are evil
by Anonymous Monk on Nov 25, 2003 at 17:23 UTC
    If we later decide to change the implementation of our list to a hash our loop will also process the hash keys (this would most likely not be what we want). We have to change code outside our object to conform with the new internal data structure of the object.

    No, you just need to change your accessor so that it returns the values instead of the whole hash. You incur the cost of building an array where you could just return a premade one before, but it's dangerous to return references to your internal data structures anyway (you lose control of what's in them). Being able to make this kind of change is most of the argument behind using accessors instead of data members in the first place.

    I agree that accessors can be misused, but I'm a little worried that people are going to interpret your advice as meaning that you should always use an iterator or visitor pattern instead of returning an array. In perl, that's not necessary.

Re: Why get() and set() accessor methods are evil
by hardburn (Abbot) on Nov 25, 2003 at 17:30 UTC

    I had a short flame war with an Anony Monk about this not long ago (starts here). I'm now starting to come around to this point of view. One of my arguments was that Class::DBI is pretty much nothing but a fancy system for automatic get/set creators.

    In this article, I had orginally taken the author's point of view as a puritanical stance against any method that matches / ( get | set ) /x. After reading the "Why getter and setter methods are evil" article (linked to above), where the author explains his views more fully, I realized that it's not that accessors and mutators are problems in themselves, but ones that return primitives (strings, integers, etc.) are. Returning complex objects is something you do all the time, and it doesn't matter if the method happens to have the word 'get' or 'set' in it. On occasion, it's necessary to return primitives, but you can do it with objects more often than not.

    In a recent project I was given that was going to use Class::DBI, I realized that many of the fields we use could take objects, using the inflate/deflate params to has_a. Have a date field? It should take a DateTime object (or one of Perl's other date/time objects). Storing a URI? It should take a URI object.

    In this project, one of our tables has 22 fields. Of that number, 13 take objects (5 of which are relations to other Class::DBI objects), and 1 is the primary key (which is a primitive integer out of necessity).

    Of the remaining 8, three are boolean fields--can't do a lot with that, and making them objects wouldn't add anything. For one of the fields, we're not sure how its going to be implemented, so it's being left alone for now. Two others are strings from user input. I suppose they could become objects, but I don't see a benifit to doing so. The last two are unsigned integers and deal with a number of hours. They could potentially become DateTime fields.

    So in all, there are three fields where making them an object would be pointless, and two others where (IMHO) it would be silly to make them objects.

    I also realized that using objects, you can use Perl's object system to limit (to an extent) what goes into a MySQL ENUM or SET column (this technique is probably worthy of a meditation on its own). For instance, say you have a column declared:

    foo ENUM( "bar", "baz") NOT NULL

    Your Class::DBI subclass could use this:

    my $foo_bar = 1; # Could store the string representation, too my $foo_baz = 2; sub FOO_BAR { bless \$foo_bar, __PACKAGE__ . '::FOO' } sub FOO_BAZ { bless \$foo_baz, __PACKAGE__ . '::FOO' } __PACKAGE__->has_a( foo => (__PACKAGE__ . '::FOO'), inflate => sub { my $f = shift; bless \$f, __PACKAGE__ . '::FOO' } +, deflate => sub { my $f = shift; $$f; }, );

    If you wanted to, you could then delcare a package the same name as this table's class with a '::FOO' at the end and add methods there. Then you put a note in your documentation so people will know to use the subroutines to get the data to go in that column.

    ----
    I wanted to explore how Perl's closures can be manipulated, and ended up creating an object system by accident.
    -- Schemer

    : () { :|:& };:

    Note: All code is untested, unless otherwise stated

      I realized that it's not that accessors and mutators are problems in themselves, but ones that return primitives (strings, integers, etc.) are. Returning complex objects is something you do all the time, and it doesn't matter if the method happens to have the word 'get' or 'set' in it

      Just a point of clarification: Get/Set methods are a short way of labelling methods that access an objects attributes, that is, methods that are only concerned with reading or writing object state directly. It has nothing to do with whether the value of that slot is a primitive or an object, but whether it represents object-state. Objects should rarely provide methods to the outside world that are only concerened with accessing state.

      hardburn wrote: Two others are strings from user input. I suppose they could become objects, but I don't see a benifit to doing so.

      While I don't think you meant to, you actually touched on a fundamental point in favor of OO programming: create an object when you have invariants that must be respected.

      If the user can enter any arbitrary string, then there's not much point in using an object. However, if the string must be validated in some way (such as with a URI), then creating an object is perfect. At the very least, I suspect that your strings might have a length limit. If that is the only invariant that you need to worry about, perhaps it's not worth the overhead of creating an object (particularly since you'll likely be validating length via your Class::DBI objects). If you have any more invariants, then using an objects can be a great way to ensure that the data is what you intended it to be.

      Cheers,
      Ovid

      New address of my CGI Course.

Re: Why get() and set() accessor methods are evil
by inman (Curate) on Nov 25, 2003 at 17:30 UTC
    The key to all things OO is encapsulation. Basically you publish an interface which people then use. If you start changing the behaviour of the interface then you are going to cause problems. The thing to remember is that accessor methods are as much a part of the encapsulation as constructors or other more conventional methods and should be treated with the same respect. The problem that has been highlighted is that the lazy implementation of get/set functions as a thin layer over the top of the internal representation is brittle and may cause a maintenance headache.

    In your example you have a method (sub) called get_array. You have published an interface that says 'Whenever you invoke this method you will get an array to work with'. This is in effect a contract between you, the designer of the class and whoever uses your class. When the internal representation of the data changes to a hash, the get_array method will end up doing more work since it now needs to construct an array to pass back to the caller. Thus maintaining the contract and not causing problems.

    inman

      This is exactly the problem I was trying to bring up ( much better worded by you). If you don't take care with how you use accessors making changes later can force you to change your interface. In general I think a good rule of thumb is to try an avoid accessor methods. However, I can see a few cases where you do need to use public accessors (the CGI module's param() method and some templating modules for example).
Re: Why get() and set() accessor methods are evil
by pg (Canon) on Nov 25, 2003 at 17:47 UTC

    I think:

    • Your statement is way too generalized, and looked at the situation basically in a black and white manner, when it is not;
    • The example you gave does not strengthen your conclusion.
    • (Also this is not an OO issue, but merely a Perl issue.)

    But your example did make me think. Part of the problem your code showed is that it is not pure OO.

    Ideally, modification of internal structure shall not cause interface change, but in this case, it caused. Is this caused by using of getter and setter? NO. Now if Perl becomes more OO,

    • Imaging you no longer return list, array or hash, instead you return an OO object, a collection (a collection of object, and the collection itself is also an object.)
    • list, array and hash are now OO objects
    • The OO objects represent list, array and hash all implement collection interface.

    Now whatever you use internally, hash, array or list, the outsider world will see it in the same way (a collection that supports a given set of method, regardless how those method are IMPLEMENTED), and the setter, getter will stay the same.

    The real problem is that your code is not pure OO, or Perl does not really support you to do it in that way.

    But the defects showed by your code example does reminder people that, when you do OO design and coding in Perl, there are certain considerations special to Perl that you have to take into consideration.

      (Also this is not an OO issue, but merely a Perl issue.)

      The example might have shown a problem specific to Perl, but the idea is applicable to any language. A Java API that returns an array of integers stored in an otherwise private field would be just as problematic.

      ----
      I wanted to explore how Perl's closures can be manipulated, and ended up creating an object system by accident.
      -- Schemer

      : () { :|:& };:

      Note: All code is untested, unless otherwise stated

        You are right, as Java also has primitive types.

        But on the other hand, in this case, I can choose to return, for example, an ArrayList of Integer. This is much less a problem to languages that have stronger OO support.

        Update:

        Thanks BUU for the extension, tie was actually something I thought of. The idea of tie, actually holds some characteristics that OO interface provides.

      Thanks pg. You explained it much more clearly than I did. The last link I gave advocates returning objects instead of primitive types. This very neatly avoids the problem and moves your program closer to the "everything is an object" ideal of languages like Smalltalk.

Re: Why get() and set() accessor methods are evil
by enoch (Chaplain) on Nov 25, 2003 at 18:45 UTC
    Your example is breaking encapsulation. That is, it is avoiding the accessor and reaching in to the internals. If we have the object:
    package OOtest; sub new { my $invoker = shift; my $class = ref($invoker) || $invoker; my $self = {}; bless ($self, $class); $self->{list} = []; return $self; } sub get_array { my $self = shift; return $self->{list}; }
    To access the array, use the accessor get_array and do not bypass it by delving in and directory accessing the private variable.
    my $obj = OOtest->new(); # NOT foreach my $tmp ( @{ $obj->list() } foreach my $tmp ( @{ $obj->get_array() } ) { # process list }
    Now, internally, you want to change OOtest to use a hash, but you do not want to break other code that expects it to use (and wants) an array. Behold the beauty of accessors, you merely change the get_array subroutine.
    package OOtest; sub new { my $invoker = shift; my $class = ref($invoker) || $invoker; my $self = {}; bless ($self, $class); $self->{list} = (); return $self; } sub get_array { my $self = shift; return values %{$self->{list}}; # anything asking for an array gets + it }
    enoch
Re: Why get() and set() accessor methods are evil
by herveus (Parson) on Nov 25, 2003 at 19:10 UTC
    Howdy!

    Getters ought to be present to provide access to well defined attributes of an object. If, for example, the object measures something, I may provide a getter to allow a user of that object to extract the specific value that has been measured. One should always be stingy in offering getters. After all, once you have put it in your public contract, it's a whole lot harder to remove it.

    When an attribute is a collection, it gets more "interesting". Fowler, in "Refactoring", describes one called Encapsulate Collection. In that refactoring, you don't return the raw collection with a getter. You return a read-only version, if you must return the collection, and provide add and remove options (as appropriate) to permit modifications. That keeps changes under the control of the object.

    Now, Fowler speaks specifically to a Java audience, but the concept here is worth keeping in mind. The key is to contract to provide data in a particular format and then to keep that contract. Try to choose a format that gives you the flexibility to do what you may need to do on the inside without having to go through extra contortions to keep that contract.

    I've been doing some Java programming to help firm up the knowledge I gained in a recent class. It's been usefully interesting. I expect my Java experience to inform my Perl programming in useful ways. On the other hand, there are things Perl makes so much easier. I have not yet sorted out if and how to say "foreach my $f (fn_returning_list()){do stuff}" without the structural overhead of creating an iterator and explicitly walking it.

    yours,
    Michael
Re: Why get() and set() accessor methods are evil
by demerphq (Chancellor) on Nov 27, 2003 at 00:34 UTC

    I found this whole thread interesting. When i read the title I thought "yep, I agree with that" but then on reading closer it wasnt even what I was thinking at all! :-) What I thought you meant was all those aweful sub like

    sub get_foo { my $self=shift; return $self->{foo}; }

    and its correspondingly ugly set_foo. If you had been talking about those being evil then i would agree. :-) Perl offers fancier and IMO more intuitive ways to do things:

    sub foo { my $self=shift; if (@_) { $self->{foo}=shift; return $self; } else { return $self->{foo} } }

    (or a variant thereof, you can golf it down nicely. :-)


    ---
    demerphq

      First they ignore you, then they laugh at you, then they fight you, then you win.
      -- Gandhi


Re: Why get() and set() accessor methods are evil
by scrottie (Scribe) on Nov 27, 2003 at 07:25 UTC
    Hi,

    I wrote-up something along these lines a while ago for perldesignpatterns.com. Other things have been written in the past too. Off the top of my head, the major gripes people have with put-get accessors (not lvalue or tied interface) are:
    • -> becomes the only useful operator. + becomes foo->add() or foo.add().
    • Side effect of above: incrementing or any += style operator becomes particularly painful
    • The rich interfaces of hashes and arrays is lost when implementing datastructures as objects, an extremely common case
    Like functional programming, object oriented programming is more a state of mind than a set of tools. There are powerful motivators for people to drop the features of Perl for a far more simplistic language that offers a clean, consistent object library and strict checking. These kinds of projects are seldom done in Perl - projects with numerous programmers. Attempting to do these large projects without the natural inter-programmer boundaries that interfaces afford is every bit as painful as losing hashes, automatic stringification, and so on. Well, it need not be such a choice: operator overloading, tied interfaces, and lvalue methods collectively allow you to present a portion of an OO interface as an array, hash, or so on. An early on accepted RFC for Perl 6 (perhaps this has changed since then) asks that hashes iteractors be reset explicitly with a sort of %hash.reset() type thing. Things like exists() would be made into methods as well rather than polluting the core namespace. Hashes have a rich interface, but as rich it is, it will never be rich enough. Your object will present some of its interface masquerading as core Perl features, but after a point, you must commit to an API, and you should do this carefully, thoughtfully, and knowledgeably. This is a topic unto itself. Designing anything that pases the test of time boils down to becoming a history major, studying many falls of many civilizations. In other words, just because you can avoid OO interfaces for a while doesn't mean you should forever, or that you need be any less skillful with them just because you're writing Perl instead of Java.

    -scott
Re: Why get() and set() accessor methods are evil
by bronto (Priest) on Nov 28, 2003 at 19:59 UTC

    Hello synistar

    Months ago I wrote a meditation on the involuntary encapsulation violation; even if I had three nice answers, I still think that that kind of implicit ability of modifying the internals of an object without using accessors is a bad thing. What I think is wrong in an accessor like get_list is that you are passing back to the caller a reference to an internal structure of the object. Do something like this:

    my $x = $obj->get_list ; $x->[3] = 'surprise!' ;

    ...et voilà!: you changed the object without using set_list. I agree that the example above is really-bad-code, but there is a lot of it around...

    Strictly speaking about your meditation, I like best accessors that have the ability to get/set their values, or read-only ones, or write-only ones. After reading Advanced Perl Programming I started using get/set methods, but after a short time I found them really annoying...

    Ciao!
    --bronto


    The very nature of Perl to be like natural language--inconsistant and full of dwim and special cases--makes it impossible to know it all without simply memorizing the documentation (which is not complete or totally correct anyway).
    --John M. Dlugosz

Log In?
Username:
Password:

What's my password?
Create A New User
Node Status?
node history
Node Type: perlmeditation [id://309952]
Approved by broquaint
Front-paged by bart
help
Chatterbox?
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others musing on the Monastery: (17)
As of 2014-09-02 13:32 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    My favorite cookbook is:










    Results (22 votes), past polls