Beefy Boxes and Bandwidth Generously Provided by pair Networks
XP is just a number
 
PerlMonks  

Make your classes use their own methods

by petdance (Parson)
on Nov 24, 2003 at 03:35 UTC ( [id://309419]=perlmeditation: print w/replies, xml ) Need Help??

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.

xoxo,
Andy

Replies are listed 'Best First'.
Re: Make your classes use their own methods
by Anonymous Monk on Nov 24, 2003 at 03:56 UTC

    I would make the stronger statement: No attribute shall be accessed except via a get/set method, and that access should itself be limited to within the class in the vast majority of cases.

      "I would make the stronger statement: No attribute shall be accessed except via a get/set method"

      I agree with this one, and it should actually be extended to cover access from within the class itself. That's actually the point the original post is trying to make.

      "that access should itself be limited to within the class in the vast majority of cases"

      I disagree with this. If I understand this correctly, you mean that those getters and setters shall not be used by other classes. This is way too general. We are not making black boxes. In fact:

      • If the attribute shall be observable from outside, the getter/setter for that attribute shall be publicly accessible (Update: I modified the last two words to fix language problems. I realized the mistakes after I read the anonymous reply. Thanks, Anonymous Monk ;-).
      • If the attribute is only used for program itself for programing reasons, yes, its getter/setter shall be restricted to the class itself.

      There is a very subtle difference here, some attributes are real attributes that descibe the characteristics of the class, in this sense, the meaning of attribute matches its real world meaning. The setters/getters of those attributes shall be publicly accessable (although the variable represents the attribute shall be private), so that other classes instantiating this class, can understand it, measure it, observe it.

      There is another type of "attributes", which are really just global variables within the class itself (most of the time, introduced by the programming of the class, but not the class itself), and no other class actually need to know them, then not just the variable itself, but its getter/setter shall be private.

      Also, getter and setter of the same attribute could have different access level. Some of the attributes could be readable, but not writable from outside, so their getters are public, but setters are private.

      Also, in some languages, for example Java, there is a protect access level in between private and public, so that some methods (obviously including setters/getters) are only accessible from certain classes, but not all classes. That makes a lot sense to me.

        If the attribute shall be observable from outside, the getter/setter for that attribute shall be public acessable.

        I agree. I meant that far too often, public getter/setter methods are a sign of poor OO design: objects as datastructures rather than objects as behaviors/responsibilities and state.

      Although I am inclined to agree with some of the points made I would not make a blanket generalization about accessor methods like that. They can be detrimental to your design.

      This article at the javaworld website enumerates the argument against get/set methods. It goes even further than I would in arguing against accessor methods but I feel that it brings up some important points.

      Here is a quote from the article that gives you an idea of the angle it takes:

      "Getter and setter methods (also known as accessors) are dangerous for the same reason that public fields are dangerous: they provide external access to implementation details. What if you need to change the type of the accessed field? You also have to change the accessor's return type. This return value is used in numerous places, so you must also change all of that code. I want to limit the effects of a change to a single class definition. I don't want them to ripple out into the entire program.
      Since accessors violate the principle of encapsulation, you can reasonably argue that a system that heavily or inappropriately uses accessors simply isn't object oriented."

      The simplest example of this is changing an internal data structure from an array to a hash. If you have get and set methods for it you have to go out to every place in your code where you pulled the array out with a get() method (and put it back with a set() call) and fix the code to handle a hash. This violates the data encapsulation principle of OO.

        I think you have missed the distinction made about internal vs external access. That java-world article is talking about get/set methods that are exposed to the world outside of the object. Within an object/class, accessors are the preferred way to manipulate attribute data, and this thread has been discussing access within the class.

Re: Make your classes use their own methods
by Anonymous Monk on Nov 24, 2003 at 09:19 UTC
    Classes need to use their own methods inside the class, and not access the internal object store directly, even though Perl allows it.
    "Perl" only allows it if you allow it (heard of "inside out objects"?).
      "Perl" only allows it if you allow it (heard of "inside out objects"?).

      "Inside out objects" do not prevent direct access to the attribute data within the class!

        "Inside out objects" do not prevent direct access to the attribute data within the class!

        While I think that the AM missed the point Andy was making they are correct on the inside out object front. You can use inside out objects to prevent a class from having global access to attribute data. For example:

        package MyClass; { # we make an attribute hash private to the foo method my %foo; sub foo { my $self = shift; $foo{$self} = shift if @_; return $foo{$self}; }; }; sub bar { my $self = shift; # so we can only access %foo using method foo here print $self->foo; };

        However, regardless of whether you implement your accessors with inside out objects or normal blessed hashes, Andy's point about being consistant with your use of accessors is a very good rule of thumb. It's certainly my policy.

        If you need to make your attribute setters/getters public (and that's often a big "if" as others have pointed out) then using them consistantly is a good idea for all the reasons Andy mentioned in the OP.

        Of course with a sane language design like Eiffel (or, I think, Perl 6 ;-) the syntax for attribute access and method calls is identical so the whole issue just disappears.

      Fine, fine, change "Perl" to "the way people usually do objects in Perl, with a blessed hash."

      xoxo,
      Andy

Re: Make your classes use their own methods
by Jenda (Abbot) on Nov 24, 2003 at 18:39 UTC

    Worry only about the things that you have to implement.

    1. Sometimes inheritance simply doesn't make sense. At least at the moment. Why pay the price now if it's easy to change the code later?
    2. Again, this is quite often a fairly safe asumption.
    3. If you do not write tests for the other methods you'll never know whether the other methods AND the accessors are OK anyway.
    4. How often do you change the implementation of your objects? And do you really expect this to be an "automated" transition?
    5. You should be retrofiting your code from time to time anyway. And again how often do you go around changing naming conventions?

    If your object is part of an OO hierarchy, go ahead and use accessors even inside the object, if not ... don't worry, you can change it later.

    Jenda
    Always code as if the guy who ends up maintaining your code will be a violent psychopath who knows where you live.
       -- Rick Osborne

    Edit by castaway: Closed small tag in signature

      I certainly am not saying that you should always have accessors. What I AM saying is that if you DO have them then you should ALWAYS use them. They are no more difficult to use than direct member access.

      xoxo,
      Andy

        Well sometimes they are a bit harder to use

        $selt->{count}++; $selt->count++; # I guess that's not gonna fly. # Except maybe if you do something spicy with LHS subroutines.
        But that's not the point. They are much slower. Therefore even if you force (politely ask) the outside world to use the accessors you may not want to use them inside the class. Of course you should be aware of the consequences :-)

        Jenda
        Always code as if the guy who ends up maintaining your code will be a violent psychopath who knows where you live.
           -- Rick Osborne

        Edit by castaway: Closed small tag in signature

      What if someone decides that he wants to inherit your class, even though you didn't plan that? Personally I believe users of my module to be competent and not seldom more competent coders than I so there's a great risk I'll overlook something that at least some users will see. So I try to not paint myself into corners if I easily can avoid it. This applies to more than inheritance and accessors.

      Why pay the price now if it's easy to change the code later?

      For a user that wants to extend your module it's not easy. In an ideal world the user would send you a patch and then write his extention, but we're not quite there yet...

      One should be nice to one's users! (At least if it hardly costs you anything.)

      ihb
Re: Make your classes use their own methods
by fletcher_the_dog (Friar) on Nov 24, 2003 at 18:59 UTC
    There is only one reason to not use your own accessor methods, and that's to microoptimize your code for speed.
    Just to see the speed difference between direct vs. method access, I did a little benchmark. If speed is important and you are using accessors repeatedly in a tight loop, there certainly is merit to direct access.
    use strict; use Benchmark "cmpthese"; my $foo = new foo; cmpthese(-5,{ direct=>sub{ $foo->{bar} }, accessor=>sub{ $foo->bar; } } ); package foo; sub new { my $class = shift; my $self = { bar=>"I am a bar" }; bless $self,$class; } sub bar{ my $self = shift; if (@_) { return $self->{bar} = shift; } return $self->{bar}; } __OUTPUT__ Benchmark: running accessor, direct, each for at least 5 CPU seconds.. +. accessor: 6 wallclock secs ( 5.08 usr + 0.01 sys = 5.09 CPU) @ 99 +4059.72/s (n=5059764) direct: 4 wallclock secs ( 5.07 usr + -0.01 sys = 5.06 CPU) @ 67 +95643.87/s (n=34385958) Rate accessor direct accessor 994060/s -- -85% direct 6795644/s 584% --
      Certainly there can be severe overhead in calling functions unnecessarily. Your example accessor is also not optimized very well: It's doing both read and write, and checking for the difference.

      More important is that the programmer know that her use of accessors, in her specific program, is a specific bottleneck. Sure, you might be using accessors in a 100-iteration tight loop, but if you're doing a fetch across the network in between calls to the loop, then that tight loop which may be "inefficient" on its own may be a trivial percentage of execution time.

      Rule 1: Premature optimization is the root of all evil.
      Rule 2: Unmeasured optimiziation is always premature.

      xoxo,
      Andy

        The only question left is whether not using accessor methods within the class should be called "optimization". Sure it improves the speed, but ... does it make the code harder to read? Does it make the class harder to use? Does it make the maintenance any harder?

        The only thing it does is that it forces you to make changes to the code should you need to use the code in some unexpected way. If you never need to do so ... you gained a lot for nothing, if you do, you only need to do a very reasonable amount of work to support the new use.

        You may need to use your class over the network one day, shouldn't it be done and used as a SOAP service even now?

        Update: Thinking about this some more ... maybe what petdance suggests could sometimes be seen as "premature generalization" :-)

        Jenda
        Always code as if the guy who ends up maintaining your code will be a violent psychopath who knows where you live.
           -- Rick Osborne

        Edit by castaway: Closed small tag in signature

Re: Make your classes use their own methods
by jonadab (Parson) on Nov 25, 2003 at 13:59 UTC
    sub name { my $self = shift; return $self->{name}; }

    This seems wrong to me. If you have a method whose sole purpose is to retrieve a property, you have bad OO design, either on the part of the language or the class. My guess is, both. (Hopefully, Perl6 will fix the language part.)

    Let me talk about the language first. It shouldn't be necessary to write a method just for that; it should be possible to just expose the property directly, and later if it needs to have more complex behavior added, transform it into a method without the need to change any external code that uses it. Inform programmers do this sort of thing with their objects all the time -- take a description property that was a string formerly and, to give it more complex behavior, turn it into a routine. External code does not need to know or care that the description is now a routine instead of a simple string; the result, from the perspective of something outside the object, is the same. Thus, above, the name property shouldn't need an accessor method; the scalar property should just be exposed directly; only if more complex behavior is needed should it be turned into a method. I believe the only reason you would ever write an accessor method is because the language lacks any way for external code to call a property without caring whether it's a scalar or a routine that returns a scalar.

    Now, if the language doesn't provide these facilities, then I can understand the temptation to make everything go through an accessor method, so that everything does not have to change if complex behavior is later added that makes the method necessary. Still, I think the added baroqueness of code created by having a lot of methods that don't actually do anything except set or return a value is in many cases more trouble than it's worth. I've been known to do it, and I don't know that it's inherently wrong per se, but I have some strong reservations about making claims to the effect that things should *always* be done this way; it sounds like something a misguided C++ programmer would say, who doesn't know any better because he hasn't learned any other paradigms yet. I believe there may be cases where it's cleaner to expose the scalar properties directly, if it can be determined that they're definitely going to remain as simple scalars in future versions of the class. This can make for simpler code in some cases, and simpler code is easier to maintain, all else being equal.


    $;=sub{$/};@;=map{my($a,$b)=($_,$;);$;=sub{$a.$b->()}} split//,".rekcah lreP rehtona tsuJ";$\=$ ;->();print$/

Log In?
Username:
Password:

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

How do I use this?Last hourOther CB clients
Other Users?
Others imbibing at the Monastery: (4)
As of 2024-04-19 16:47 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    No recent polls found