I've been trying to maintain some legacy code that badly needs updating. One of the main sins committed in this code is direct object access, so $obj->{attribute} = 'value', rather than $obj->set_attribute('value');
To fix this, I need to choose a way to implement accessors. There are a few problems. Firstly, the 'perl' way of doing this seems to be a moving target. A few years ago, people used Class::MethodMaker, but the essential (in my opinion) 'new_with_init' option has gone. Now, people seem to be choosing either Class::Accessor. Of course, what's really trendy now is inside-out objects, and the module of choice (because it's mod_perl compatible) is Object::InsideOut.
I like Object::InsideOut. I like the interface. The problem is, because I'm using legacy code, I will need to assume the objects are blessed hashes for some time yet. Object::InsideOut wants the objects to be blessed scalars and croaks if they are anything else.
Class::Accessor's new method at least returns a hash. At the moment it's probably what I'll have to use. But it doesn't automatically sanity check. For example:
package GoodbyeThere;
use base 'Class::Accessor';
__PACKAGE__->follow_best_practice;
__PACKAGE__->mk_accessors(qw/a b c/);
use strict;
use warnings;
sub do_stuff {
my $self=shift;
$self->set_b('ssss');
$self->set_c('tttt');
print "Output " . $self->get_a . $self->get_b. $self->get_c . "\n\n";
}
1;
If you go:
perl -MGoodbyeThere -e '$h = GoodbyeThere->new({a => 1}); print "$h\n";$h->do_stuff'
The output is:
GoodbyeThere=HASH(0x814f750) Output 1ssssttttSo far so good. But if you go:
perl -MGoodbyeThere -e '$h = GoodbyeThere->new({a => 1, query => 9}); print "$h\n";$h->do_stuff; print $h->{query}'
The output is:
GoodbyeThere=HASH(0x81b9d44) Output 1sssstttt 9See 'query' is still there. It would be better if it just threw it away. Which brings me to another point. With Class::Accessor, 'super' doesn't work as you might expect, so you need to write an accessor that sanity checks the input hash. I suppose there are worse things in life though...
I've been trying to maintain some legacy code that badly needs updating. One of the main sins committed in this code is direct object access, so $obj->{attribute} = 'value', rather than $obj->set_attribute('value');
Is this actually broken? In that, is this direct access causing you sufficient hassles that it's worth the code churn to change it?
Change means the potential for breakage, and clouds the revision history given by your version control change annotation. There is a great temptation as the first act to reformat all code and update to current standards, which seems really good in the short term, but there's a long term cost that is only revealed later, by which time it's too late to repent.
Re:Is it broken?
TheNomad on 2006-12-06T13:53:18
Is this actually broken? In that, is this direct access causing you sufficient hassles that it's worth the code churn to change it?It's caused a little bit of trouble as some coders have lazily stored values in the objects that shouldn't be there. It took me ages to debug these things (although it's easy now I know about them). If they'd been forced to used accessors the design would have been tweaked and would be much better as a result.
Anyway, I don't intend to change all the code at once. Instead, any new functionality or any rewrites should use the accessors. Old code will be updated to accessors when it comes up. The code will have to go through a massive refactoring reasonably soon. This will inevitably break lots of things. I may as well take the opportunity to switch over to accessors. But it means I need to plan how to implement the accessors now.
Re:Is it broken?
Abigail on 2006-12-06T15:22:12
Instead, any new functionality or any rewrites should use the accessors.
Is that a requirement specifically for this project, or do you always want this?
I don't often write accessors. I only write an accessor if I explicitely want to expose an attribute to the outside world. Otherwise, I won't provide an accessor. The problem, IMO, is that there's no such thing as private methods - any method is automatically part of the API. Which means that whenever your removing an attribute that has an accessor from your implementation, your API changes in an non-backwards compatible way.
Which is why I usually don't use accessors, but use the attributes directly. But since I'm using Inside-Out Objects, I cannot have the problem of willy-nilly added attributes - they all have to be declared in the class.
Re:Is it broken?
DAxelrod on 2006-12-06T17:26:55
The problem, IMO, is that there's no such thing as private methods - any method is automatically part of the API.
I disagree that this is always the case.
Your API is what you document it to be. It's perfectly valid to either document "private" methods elsewhere, or put them in a part of the documentation that has the disclaimer "The behavior of the following is unspecified. This is how these might work now, these are all subject to change without notice, I guarantee nothing."
I suspect that you know that, and are instead arguing "In practice, people will use methods as if they were a documented part of the API."
Re:Is it broken?
Abigail on 2006-12-07T08:42:15
I disagree.
The problem is just as I said. There are no private methods. Suppose class 'A' defines a method 'foo', and class B inherits A. No matter how often A documents 'foo' to be private, B is robbed from having a method 'foo' and do away with it freely. That's the essense of not having private methods - all methods will be inherited by a subclass. Programmers (can) read the documentation, but perl will not.
Re:Is it broken?
DAxelrod on 2006-12-07T15:06:30
Ah, OK. I concede that that's a good consideration.Re:Is it broken?
TheNomad on 2006-12-06T20:40:13
Is that a requirement specifically for this project, or do you always want this?It's for this project. There are good reasons why accessors are needed for the project.
However, generally I'm lazy and can't be bothered with any sort of accessors. And in small projects direct access hasn't created any problems.
When I first heard about inside-out objects, I thought they were solving problems that hardly ever arose. I've become a convert though!
You don't have to have a blessed hash; you only need something that supports hash dereferencing. (Of course, then you have the joy of fixing all of the code that assumes that ref returns something meaninful.)
Re:overload
TheNomad on 2006-12-06T20:32:14
You don't have to have a blessed hash; you only need something that supports hash dereferencing. (Of course, then you have the joy of fixing all of the code that assumes that ref returns something meaninful.)
Brilliant! I might have a play with that and see what I manage to come up with.Re:overload
chromatic on 2006-12-07T08:27:40
Perl Hack #87 has another trick, this time using restricted hashes to catch people poking in object innards unwisely.
"But, wait!", you cry. "Accessing the attribute like that is an encapsulation violation!"class Foo
attr_accessor:attribute
end
obj = Foo.new
obj.attribute = 'value'
You're not accessing the attribute directly. The syntax just makes it *look* like you are. In reality you're calling the Foo#attribute= method. This longhand example should clarify things:
class Foo
attr_reader:attribute
def initialize
@attribute = 7
end
def attribute=(value)
puts "Hello World!"
end
end
obj = Foo.new
puts obj.attribute # 7
obj.attribute = 99 # "Hello World!"
puts obj.attribute # 7 - wasn't touched
Re:Pimping Ruby
chromatic on 2006-12-10T17:34:15
def initialize
@attribute = 7
endThis is one of those areas where Perl 6's Uniform Access Principle demonstrates just how clunky and non-uniform Ruby's instance attributes are.
Re:Pimping Ruby
djberg96 on 2006-12-10T19:33:58
I assume you mean the fact that Perl 6 provides default read-only (though configurable) accessors on attributes. That's what I get from Exegesis 12, anyway, unless something has changed.Personally, I think it's a misfeature, because I don't always want users to have access to the instance variables I'm using. I'm sure that's configurable, too, but I'd rather be explicit than implicit in this case. Or, just write a custom 'is' method that lets me imitate the Perl 6 syntax.
The topic of autogenerating accessors for instance variables has come up on ruby-talk from time to time, and I think you can get third party libraries that do this (presumably by overriding Class#initialize). The fact that there has never been a clamoring for it among the Ruby community ought to be telling, though.
Re:Pimping Ruby
chromatic on 2006-12-10T22:12:28
I actually meant that most of the Ruby code I've seen accesses instance variables directly within the class, despite the presence of an autogenerated accessor. There's a subclassing problem there. (Actually it's not only a problem for subclassing, but I use that as a convenient shorthand because I don't have a good way to say "allomorphic instance modification".)
The fact that there has never been a clamoring for it among the Ruby community ought to be telling, though.I've never seen a clamoring for a saner
requireeither, but I don't take that as evidence that it's a particularly clear, clean, or well-defined feature.Re:Pimping Ruby
djberg96 on 2006-12-11T13:39:01
I actually meant that most of the Ruby code I've seen accesses instance variables directly within the class, despite the presence of an autogenerated accessor. There's a subclassing problem there.I'm afraid I don't follow. Unless you have a custom writer, doing "self.attribute = 'value'" instead of "@attribute = 'value'" is a wasted method lookup. I fail to see what subclassing problem you would have.I've never seen a clamoring for a saner require either, but I don't take that as evidence that it's a particularly clear, clean, or well-defined feature.My only gripe with require is that it doesn't look in its own directory first, and so all the library authors end up doing $LOAD_PATH.unshift(File.dirname(__FILE__)) in each file. Otherwise, I'm not sure what you're referring to.Re:Pimping Ruby
chromatic on 2006-12-11T18:42:24
I fail to see what subclassing problem you would have.It's only a problem if a subclass wants to do something different with an attribute. I run into that once in a while.
Re:Pimping Ruby
djberg96 on 2007-01-03T21:04:57
Can you provide an example that demonstrates the issue?
Object::InsideOut wants the objects to be blessed scalars and croaks if they are anything else.
Object::InsideOut does way too many things and breaks a major principle of inside-out objects (the exact one that is causing you to disregard it). Take a look at Class::InsideOut instead, which does only the minimum needed for a solid, proper inside-out implementation.