•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.
| [reply] [Watch: Dir/Any] |
|
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
| [reply] [Watch: Dir/Any] |
|
| [reply] [Watch: Dir/Any] [d/l] [select] |
|
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 things 1 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.
| [reply] [Watch: Dir/Any] [d/l] [select] |
|
| [reply] [Watch: Dir/Any] |
|
|
|
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 | [reply] [Watch: Dir/Any] [d/l] [select] |
|
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).
| [reply] [Watch: Dir/Any] |
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!
| [reply] [Watch: Dir/Any] |
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
| [reply] [Watch: Dir/Any] [d/l] [select] |
|
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.
| [reply] [Watch: Dir/Any] |
|
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.
| [reply] [Watch: Dir/Any] |
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.
| [reply] [Watch: Dir/Any] |
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.
| [reply] [Watch: Dir/Any] |
|
(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
| [reply] [Watch: Dir/Any] [d/l] |
|
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.
| [reply] [Watch: Dir/Any] |
|
|
|
| [reply] [Watch: Dir/Any] |
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 | [reply] [Watch: Dir/Any] [d/l] [select] |
Re: Why get() and set() accessor methods are evil
by herveus (Prior) 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.
| [reply] [Watch: Dir/Any] |
Re: Why get() and set() accessor methods are evil
by eric256 (Parson) on Nov 25, 2003 at 17:19 UTC
|
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) :)
| [reply] [Watch: Dir/Any] [d/l] |
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
| [reply] [Watch: Dir/Any] [d/l] [select] |
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
| [reply] [Watch: Dir/Any] [d/l] [select] |
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 | [reply] [Watch: Dir/Any] |