Make your classes use their own methods

petdance on 2003-11-24T03:32:07

Classes need to use their own methods inside the class, and not access the internal object store directly, even though Perl allows it. This is often called "eating your own dog food." For example, if you have

sub name { my $self = shift; return $self->{name}; } then in your other methods in the class, go ahead and use the accessor directly, with sub foo { my $self = shift; print $self->name; } instead of sub foo { my $self = shift; print $self->{name}; }

There are a number of reasons for this, and you don't have to be a Java object purist to appreciate them:

  • You can't use inheritance.
    What if you have a subclass of your class, and it overrides the name() method, but not the foo() method? You won't be using the overridden name()
  • You're making a permanent assumption that the accessor will always be simple
    What if somewhere down the road you do some manipulation inside name()? Say you want it to return a proper-cased string. Now you're going to either replicate that code to proper-case the string, or you're going to go and change your uses of $user->{name} to $user->name anyway.


  • Using your own accessors tests them
    What if your accessor is broken somehow? You'll never know.


  • You're tying your code to the specific implementation of your object.
    Objects don't have to be hashes: Maybe you want to change to use an array, like in WWW::Mechanize::Link.


  • You're tied to the naming convention
    Maybe you want to change the internal naming convention. You shouldn't have to go retrofit your code to do so.


Heck, accessors can even reduce punctuation: $user->name vs. $user->{name}.

There is only one reason to not use your own accessor methods, and that's to microoptimize your code for speed. If you've gone ahead and profiled your code, say, with Devel::DProf, and you know that calls to your accessors are causing a significant slowdown from overhead, and you've eliminated other sources of speedup, and you comment your code explaining WHY you're using the attributes directly, then sure, go ahead. We had to do this in MARC::Record in some tight-loop intensive code, but even then we changed some internal representations that bypassed the accessors entirely to get even more speedup.


Agreed!

chromatic on 2003-11-24T05:55:38

I've recently changed over to doing this in my own code and it's quite nice. You can really tell how complex a method is by how many accessor methods you need to call on it.

At the very least

jdavidb on 2003-11-24T06:20:52

At the very least, write good tests for all your public methods. Then, when you are finally coerced into changing your attribute accesses into method calls as Andy describes (and you will if any of the following is true: your code lives long enough, your code grows large enough, or your code passes through enough maintainers), making the change will be much quicker, easier, and reliable due to the confidence you receive from being able to run the tests and say "It runs exactly like it did before I did that global search and replace."

Effective unit testing reduces the cost of change.

But Be Careful

davorg on 2003-11-24T11:20:35

I was recently persuading a cow orker on this very point and changed all of the attribute accesses in the constructor in order to demonstrate.

Unfortunately we didn't bless the hash reference into the object until the end of the constructor so none of the accessor calls worked.

What started off as a demonstration of why using accessors is a good idea turned into a demonstration of why unit tests are essential before doing any refactoring :)