Fellow monks, I've been thinking about access to my fields and came up with the following.
I'm thinking of publishing this little work in the tutorials section if enough feedback warrants it of course :)
Working with public, private, and protected fields
Introduction
Many perl objects are created by using hash references, be it the normal perl OO way of doing things our by using Inside-Out classes. However, I prefer using closures because I can define private and protected fields with ease.
[nodeId:8251] provides a nice introduction to closures and talks about private fields. I suggest you read [nodeId:8251] first to get a short introduction in closures and private access.
What does public, private, and protected mean actually
A few definitions
- public
- A public field is accessible anywhere.
- private
- The field is accessible only within the class that defines it.
- protected
- The field is accessible only within the class that defines it and within subclasses
Our base class
The class Duck defines the basic behaviour of a Duck. A duck can fly and/or quack. See [nodeId:533592] for more information
./src/bo/Duck.pm
package src::bo::Duck;
use strict;
use warnings;
use Carp;
sub new {
my $class = shift;
my $self = {
FLYBEHAVIOUR => { 'VALUE' => undef, 'ACCESS' => 'protected' },
QUACKBEHAVIOUR => { 'VALUE' => undef, 'ACCESS' => 'protected' },
};
my $closure = sub {
my $field = shift;
$self->{$field}->{'ACCESS'} eq 'private'
and caller(0) eq __PACKAGE__
|| confess "$field is private";
$self->{$field}->{'ACCESS'} eq 'protected'
and caller(0)->isa(__PACKAGE__)
|| confess "$field is protected";
if ( @_ ) { $self->{$field}->{'VALUE'} = shift; }
return $self->{$field}->{'VALUE'};
};
bless ($closure, $class);
return $closure;
}
sub setFlyBehaviour {
caller(0)->isa(__PACKAGE__) || confess "setFlyBehaviour is protected";
my $closure = shift;
my $flyBehaviour = shift;
&{ $closure }("FLYBEHAVIOUR", $flyBehaviour);
}
sub setQuackBehaviour {
caller(0)->isa(__PACKAGE__) || confess "setQuackBehaviour is protected";
my $closure = shift;
my $quackBehaviour = shift;
&{ $closure }("QUACKBEHAVIOUR", $quackBehaviour);
}
sub doFly {
my $closure = shift;
&{ $closure }("FLYBEHAVIOUR")->fly();
}
sub doQuack() {
my $closure = shift;
&{ $closure }("QUACKBEHAVIOUR")->quack();
}
1;
What have I done to have private and protected fields
FLYBEHAVIOUR and QUACKBEHAVIOUR are protected fields. These fields are only accesible within Duck.pm are the classes that subclass Duck.pm.
FLYBEHAVIOUR and QUACKBEHAVIOUR are implemented as simple hashes. Both fields have two hash entries. The entry for 'VALUE' specifies the value of our fields while 'ACCESS' specifies private and/or protected access.
my $self = {
FLYBEHAVIOUR => { 'VALUE' => undef, 'ACCESS' => 'protected' },
QUACKBEHAVIOUR => { 'VALUE' => undef, 'ACCESS' => 'protected' },
};
How do we archiieve private and protected field access
Our constructor is a bit special. It contains a closure that gets blessed as a class of the type Duck. The closure does all the checking for field access.
my $closure = sub {
my $field = shift;
$self->{$field}->{'ACCESS'} eq 'private'
and caller(0) eq __PACKAGE__
|| confess "$field is private";
$self->{$field}->{'ACCESS'} eq 'protected'
and caller(0)->isa(__PACKAGE__)
|| confess "$field is protected";
if ( @_ ) { $self->{$field}->{'VALUE'} = shift; }
return $self->{$field}->{'VALUE'};
};
Whenever we call our instantiated class we pass through the closure for field access. When we call a field we first check it's 'ACCESS' entry in the hash to determine if access is 'private' or 'protected'. If no 'ACCESS' is defined or something else than 'private' or 'protected' we treat the field as public.
Checking a private field is easy. We already know that when caller(0) equals the package name __PACKAGE__ we are indeed calling from our class. If however caller(0) equals something else we're calling from outside.
Checking for protected fields is likewise the same. Whenever caller(0) IS A __PACKAGE__ and the field has 'protected' access we know we can access it. If caller(0) IS something else than __PACKAGE__ we give a run time error.
Why is this important
I want to avoid access to the fields in a direct way. My class Duck.pm provides easy setter methods setFlyBehaviour and setQuackBehaviour and I want the users of my class to use these setters all of the time.
(Imagine a setter that first checks the parameters and does some intermediate calculations before setting the value of the field. You really want to make sure your users call the setter to avoid having incorrect values in your field!)
The following code should be avoided as much as possible:
$whistle->("QUACKBEHAVIOUR", src::bo::behaviour::quack::CanQuack->new() );
A closure that implements checking of access does protect against setting fields directly.
What about setters/getters
As you can see in the following piece of code it's really simple to have private and/or protected access for your setters and/or getters.
sub setFlyBehaviour {
caller(0)->isa(__PACKAGE__) || confess "setFlyBehaviour is protected";
my $closure = shift;
my $flyBehaviour = shift;
&{ $closure }("FLYBEHAVIOUR", $flyBehaviour);
}
sub setQuackBehaviour {
caller(0)->isa(__PACKAGE__) || confess "setQuackBehaviour is protected";
my $closure = shift;
my $quackBehaviour = shift;
&{ $closure }("QUACKBEHAVIOUR", $quackBehaviour);
}
The example of course gives only the code for protected access. If you want private access you should use for example the following line:
caller(0) eq __PACKAGE__ || confess "setQuackBehaviour is private";
How do I extend such a class
The class Rubber defines the properties of a rubber Duck. A rubber Duck inherits of course all properties of a base Duck but also provides a method and field to set/get the color of the rubber Duck.
./src/bo/Rubber.pm
package src::bo::Rubber;
use strict;
use warnings;
use Carp;
use src::bo::Duck;
use base qw/src::bo::Duck/;
use src::bo::behaviour::fly::CannotFly;
use src::bo::behaviour::quack::CanSqeek;
sub new {
my $class = shift;
my $extends = $class->SUPER::new( @_ );
$extends->setFlyBehaviour( src::bo::behaviour::fly::CannotFly->new() );
$extends->setQuackBehaviour( src::bo::behaviour::quack::CanSqeek->new() );
my $self = {
COLOR => { 'VALUE' => undef, 'ACCESS' => 'protected' },
};
my $closure = sub {
my $field = shift;
if ( exists $self->{$field} ) {
$self->{$field}->{'ACCESS'} eq 'private'
and caller(0) eq __PACKAGE__
|| confess "$field is private";
$self->{$field}->{'ACCESS'} eq 'protected'
and caller(0)->isa(__PACKAGE__)
|| confess "$field is protected";
if ( @_ ) { $self->{$field}->{'VALUE'} = shift; }
return $self->{$field}->{'VALUE'};
}
else {
return $extends->($field,@_);
}
};
bless ($closure, $class);
return $closure;
}
sub setColor {
my $closure = shift;
my $color = shift;
&{ $closure }("COLOR", $color );
}
sub getColor {
my $closure = shift;
&{ $closure }("COLOR");
}
1;
Our constructor is of course new:
sub new {
my $class = shift;
my $extends = $class->SUPER::new( @_ );
$extends->setFlyBehaviour( src::bo::behaviour::fly::CannotFly->new() );
$extends->setQuackBehaviour( src::bo::behaviour::quack::CanSqeek->new() );
my $self = {
COLOR => { 'VALUE' => undef, 'ACCESS' => 'protected' },
};
my $closure = sub {
my $field = shift;
if ( exists $self->{$field} ) {
$self->{$field}->{'ACCESS'} eq 'private'
and caller(0) eq __PACKAGE__
|| confess "$field is private";
$self->{$field}->{'ACCESS'} eq 'protected'
and caller(0)->isa(__PACKAGE__)
|| confess "$field is protected";
if ( @_ ) { $self->{$field}->{'VALUE'} = shift; }
return $self->{$field}->{'VALUE'};
}
else {
return $extends->($field,@_);
}
};
bless ($closure, $class);
return $closure;
}
First we start by calling the instructor of our base class by using the SUPER::new( @_ ) method. This call to SUPER::new( @_ ) gets us the closure of the base class Duck.pm.
By calling setFlyBehaviour and setQuackBehaviour we can give values to the FLYBEHAVIOUR and QUACKBEHAVIOUR fields in our base class.
For our COLOR field we define a hash COLOR in the normal way. Note that we use the same convention as in our base class. COLOR has a 'VALUE' and an 'ACCESS'.
Our closure in our subclass looks a lot like the closure from our base class. The only actual difference is the extra check to see if we're talking about a field defined in our own class or in the base class we're extending.
If we're talking about a field in our own class we provide the same checks as in the base class. If however it's a field from the base class we delegate all calls to the closure from the base class.
if ( 1 ) { $postman->ring() for (1..2); }
Enforcing protection makes many nice hacks very hard, or even impossible. That's very unfortunate, in a highly dynamic language, in my opinion. I choose to trust the caller to know what the underscore means. If anyone wants to call my "private" or "protected" methods, they can go ahead. But they have been warned (by the underscore and/or (lack of) documentation).
Juerd # { site => 'juerd.nl', plp_site => 'plp.juerd.nl', do_not_use => 'spamtrap' }
Unfortunately, lack of documentation doesn't guarantee a feature isn't used. :-(
--
Ytrew
However, I prefer using closures because I can define private and protected fields with ease.
I appreciate that you put some effort into drafting a tutorial-style meditation. However, in both this post and A quicker way to have protected and private fields?, you show a clear bias for closure-based objects over any other type. (You do realize that node 8251 was written in 2000, right?)
At the very least, for clarity, I think that you should retitle your post to something like "Closure objects with public, private, and protected fields".
Morever, I think that your piece would be much stronger if you explored several ways to do public, private and protected fields and articulated the pros and cons of each. Some people might reach different conclusions on whether the pros and cons lead to the same place.
For example, with a closure based object of the type you articulate, every private field access (from within __PACKAGE__) requires:
- Dereferencing the closure reference
- Calling the closure
- Two hash dereferences to get the access level
- Checking the access level and the caller
- Two more hash dereferences to get the value
Contrast that with an inside-out object:
package Duck::Secret;
use Class::InsideOut qw( private id register );
private secret => my %SECRET;
sub new {
my $self = register( bless \(my $s), shift );
$SECRET{ id $self } = int( rand(100) );
return $self;
}
# details of
sub guess_secret {
my ($self, $guess) = @_;
return $guess == $SECRET{ id $self };
}
Accessing that inside-out private field requires:
- Calling id (which is really Scalar::Util::refaddr)
- One hash lookup
So, in this example, if someone has lots of private fields and few (or no) protected fields, inside-out objects seem likely to have much better efficiency.
Adding protected accessors for inside-out objects is also easy:
# continuing previous code example
sub secret {
my $self = shift;
die "Secret is a protected field" if ! caller(0)->isa( __PACKAGE__ );
$SECRET{ id $self } = shift if @_;
return $SECRET{ id $self };
}
That still has less overhead than a closure-based object.
So, maybe you'd like to consider a fuller treatement of the subject before declaring closure-based objects are the way to go.
-xdg
Code written by xdg and posted on PerlMonks is [http://creativecommons.org/licenses/publicdomain|public domain]. It is provided as is with no warranties, express or implied, of any kind. Posted code may not have been tested. Use of posted code is at your own risk.
Inside-out objects also require much less code and work. I can't see how the benefits of the blessed closure technique demonstrated in the original post make up for its lack of clarity and maintainability or its repetition.
It looks like you put considerable thought into this. I applaud your hard work. Still, I have to say that whenever I encounter some arrangement that tries to protect me against myself, I get a bit defensive or annoyed or something. I feel I should be allowed to get myself into as much trouble as I want, even if it's something very dumb and avoidable. No matter how many times I had to be told not to stick my finger into the light socket, I only had to do it once to convince myself that this wasn't something I wanted to do. Well, OK, not more than a few times ;-)
If the people who caused the problems were the same people who had to fix the problems, I might agree with you. :-) That's not always (often?) the case. I hate it when the person currently working on a project assumes that they will be the only person ever to work on the project. Especially when the person who ends up being asked to "just make it all work" when the person leaves the company is me. :-(
--
Ytrew
Perl isn't the type of language that caters to the lowest common denominator, so in my book flexibility trumps idiot proofing.
If you're clever enough to outhink the module author, surely you're clever enough to extend his module to cover your reasonable and well thought out usage pattern?
so in my book flexibility trumps idiot proofing
Flexiblity is comparatively easy; idiot proofing is very, very hard. ;-) Ask anyone who's tried to keep small children (or cats!) safe; it's harder than it sounds sometimes.
--
Ytrew
flexibility trumps idiot proofing
True. But wouldn't it be nice if we could have both?
However, you do get both to some extent by using something like the _variable convention. You are told what you should be doing, but aren't forced to do it.
To me, that is sufficient idiot proofing. I don't think its in society's best interest to protect really determined idiots.
However, you do get both to some extent by using something like the _variable convention. You are told what you should be doing, but aren't forced to do it.
But the problem is that some of these conventions, as well as allowing clever people to do clever things, mean that sensible people doing sensible things have problems.
For example if I add a _private_method to Foo module v1.01, and Sally already has a _private_method in her Foo v1.00 subclass then things will break for no good reason when Sally upgrades.
Should Sally have to go read the code before she can subclass a new version? When only "internal" APIs have changed?
Nobody is being idiotic here in my opinion. It's just Perl's broken OO encapsulation getting in the way of real work being done.
I feel your pain, and you've raised a good point. I've never worked on any sort of programmning team or project, so I've never had to clean up code, nor have mine cleaned up. I *have* come back to my own code after a few months, only to find that whatever flash of genius allowed me to create it, was no longer with me, and it was about as readable as Sanskrit ;-).
From the TMTOWTDI dept.:
My CPAN distro, KinoSearch, is a loose port of the Java search engine library Lucene. At 70+ modules, it's a large system, so enforcing access levels is important. However, watertight access level control in Perl takes heroics, and heroics are expensive both in terms of maintenance and CPU time.
The policy I came up with (text taken from KinoSearch::Docs::DevGuide), is only a mild expansion on widespread Perl practices. It's served me well so far:
No public member variables.
Multiple classes defined within a single source-code file, e.g. TermQuery and TermWeight, may use direct access to get at each others member variables. Everybody else has to use accessor methods.
C-struct based classes such as TermInfo allow direct access to their members, but only from C (of course).
Subroutine/method access levels
There are three access levels in KinoSearch.
- public: documented in "visible" pod.
- private: subs which are prepended with an _underscore may only be used within the package in which they reside -- as per perlstyle guidelines -- and in only one source file.
- distro: any sub which doesn't fall into either category above may be used anywhere within the KinoSearch distribution.
The ban on public member variables, plus some lightweight constructor argument checking, goes pretty far towards insulating KinoSearch against autovivification problems and all the rest. If there were a public API for getting at the C struct members, I'd hide them behind accessors using C function pointers, but since there's not, compile-time checking provides sufficient internal protection.
I haven't really needed protected methods. The ACL that would be handy is what they call "package" in Javaland, but I couldn't use that name for it because packages in Perl have a smaller scope, and "distro" is my imperfect substitute.
KinoSearch is currently about half/half C-struct based XS classes and hash-based classes, with a smattering of inside-out classes. I'd consider changing the internal implementation of the hash-based classes to inside-out if I thought it was important. But defining ACLs in the private API beyond what I'm doing now would be overkill, while tagging some visible methods as "protected" would just confuse a lot of Perlers.
At 70+ modules, it's a large system, so enforcing access levels is important.In my opinion, you're drawing a conclusion based on an irrelevant metric. You either need access control or you don't. Chances are that you don't, but if you feel that you need it, don't feel that you also have to justify it; it's your code. :)
thor
The only easy day was yesterday
Thor, please elaborate on why "chances are" there's no need for access control in a system that big. Perhaps you interpreted the word "enforcing" in an absolute sense? This is open source: all access control is advisory.
Exposing a member variable in a public API has the disadvantage of freezing an implementation detail. Standard OO theory says that's probably not a good idea in either large or small systems. The distinction between large and small is in the internal API -- it's more important that components in a large system do not violate encapsulation of other components.
Say you have only two small hash-based classes in your distro and one accesses a hash member in the other directly. When you refactor and change the name of that variable, you can just check the other file and be sure that there aren't any ill consequences. In a large system, that's not feasible because it's prohibitively time-consuming and difficult to inspect all the code in the project every time you change an implementation detail. However, with the ACL policy I have in place, I can change how any hash member or leading-underscore sub behaves and feel confident that I only have to check the file I'm currently editing.
thor
The only easy day was yesterday
My point is that if you use anything but the publicly defined interface, you take your life into your own hands. Make the interface to each class clean and you don't have to worry about breaking people that don't use it. I don't feel bad if I break the code of someone who relied on a non-public interface to one of my classes. Some people don't understand that you don't need things like protected or private.
Well, that depends on your definition of need.
While the _private_method naming convention is a nice signal to the reader that this particular subroutine is not part of the public API, it doesn't help with encapsulation. See node 241545 and node 247643 for some examples of where this sort of encapsulation problem can cause problems for people who are not deliberately poking at things they shouldn't be.
Sure they can go poke at the source and figure out the problem. But wouldn't it be nicer if the language stopped it happening in the first place?
(and of course this need not stop people being able to poke into the innards if they want to - just provide a separate syntax.)
Let me first say that I was once like you are, I was obsessed with trying to make Perl OO be more like other, "stricter" OO systems. I even wrote and released a module to CPAN (see [cpan://Devel::StrictObjectHash]) to do exactly what you are doing here (I used tied hashes though, instead of closures). I also wrote many "protected" and "private" methods which died if the wrong person touched them.
And then, after a few years of this, I realized that never once did I ever actually need any of this code (and the computational overhead that came with it). You see, I realized that I had just built in a bunch of checks, which would always return false, and never execute, because I always used my modules correctly. I then also realized that all my co-workers who used my code did the same. And I would venture to guess that any of the users of my CPAN modules, they also used them correctly. And why do I think this is so?
Because that is how I documented them, plain and simple.
I basically came to the realization, that this is a social problem, and not one to be solved with code. If my co-worker is violating my encapsulation, he/she is wrong. If they have to violate my encapsulation to make things work, then my design is wrong.
Now, from a purely code point of view, I also noticed something about your code (which was always in my code too), especially in how you deal with private and protected methods. You suggest that adding this at the begining of your code for a private method:
caller(0) eq __PACKAGE__ || confess "setQuackBehaviour is private";
and this for protected methods:
caller(0)->isa(__PACKAGE__) || confess "setQuackBehaviour is protected";
Both of these methods ignore the possibility that an ancestor class might implement a public version of setQuackBehaviour. Now, my Java is a little rusty, so I don't recall if that is even possible in Java, but that matters not since you are not explicitly disallowing that behavior here anyway.
To properly implement public/private/protected methods, you need to have access to Perl's method dispatcher itself, and that is, I'm sure, not possible without patching perl itself. And to really implement this kind of behavior effectively and efficiently, you probably need some degree of program analysis and code fiddling/generation, which again means a patch to perl. And by the time you are all done with this, you might as well have just saved yourself the time and used Java/C#/C++ or some other language where you get his for free.
Unfortunately, management rarely understands, nor cares, which party is "wrong" from the standpoint of computer science philosophy. Mostly, they just care whether or not the code you put into production breaks something; and if your patch to production code breaks something because you foolishly assumed your co-worker wasn't an idiot, you're going to have to answer for your mistaken assumption.
Never just assume competence on the part of other people, or even yourself for that matter, unless you have absolutely no choice. Double and triple check everything and verify that it is right.
Reliable systems come from multiple independent verifications and confirmation; not blind faith in the talents of the people working on them.
--
Ytrew
Unfortunately, management rarely understands, nor cares, which party is "wrong" from the standpoint of computer science philosophy.
Yes, I completely agree, but what I am talking about is not "computer science philosophy", but social contracts. Management does understand rules, and if "don't break encapsulation" is a rule, then the fault will be clear. Any team must have agreed upon ways of working, or you find that you will end up with total chaos. And this goes for industries outside of technology too, anyone who has ever worked in a resturant knows this. If you cook staff is not a well oiled and coordinated machine, which is governed by both written and unwritten rules, not only will peoples dinners be late/wrong, but there is a good chance someone could end up in the hospital by the time the night is over.
Mostly, they just care whether or not the code you put into production breaks something; and if your patch to production code breaks something because you foolishly assumed your co-worker wasn't an idiot, you're going to have to answer for your mistaken assumption.
Well if you are in a position of responsibility, you should always assume everyone else is an "idiot" unless proven otherwise. This is not to say you should walk around with a chip on your shoulder and look down your nose at them. Only that if it's your ass is on the line, you need to make sure your covered.
As for deploying/patching production and having things break, that is what staging/QA environments and regression test suites are for. If you are not using them, then you are already in deep trouble so what's a little bit more :P
Reliable systems come from multiple independent verifications and confirmation; not blind faith in the talents of the people working on them.
Of course, and through those "multiple independent verifications", someone would (I hope) spot the encapsulation violation and it would be addressed, and the programmer would recieve thirty lashes and placed in stocks to serve as a warning to others.
Like others in this thread I'm pretty firmly in the camp of "let the user do what they want, even if they want to do something dumb". I dont see a lot of point in enforcing privacy rules in a perl object structure. In fact, i dont see a lot of point in subroutines doing "check if I'm being used correctly" logic. And for more than one reason:
First is that such checks involve a fairly expensive overhead, second is that such checks usually just mean that instead of getting a run-time error from perl when i actually have done something wrong I get a run time error from the checker code telling me I might have done something wrong, but often with less information about what it is and why it was wrong. Lastly I find often that such checks are overly restrictive based on misunderstandings of how perl works. (The classic example is using ref to check type.)
For instance your logic for protected methods assumes that there will never need to be an interface compatible object that needs to be written that can't inherit from your base class but must work as though it does. I can work around your defense with some craftyness, but that craftyness is unnecessary if you leave off with the run-time protections and let me make my own decisions about how I use your code.
About the only time I think such logic is called for is when the action of the sub/object is "dangerous" in some way and must be used in a guarded fashion.
$world=~s/war/peace/g
Many of the replies in this thread are based around the notion that if developers willfully engages in bad practices, then it is on their heads. This is fine when you don't have to deal with the consequences. I'm currently on a project where a developer keeps wanting to access the object hashes references directly. I keep explaining that this is bad practice, but he doesn't get it.
I ride his ass at code reviews, but I really don't have any authority to do more than that, and as we're all contractors, there is no one lead developer. Now you could argue that the problem is due to the project team structure, but I would argue that it also relates to the fact that blessed hash references are used to store the object attributes. Sure, any developer who violates the public/private contract is asking for trouble, but the fact is we all suffer if things start to go wrong.
I'm currently on a project where a developer keeps wanting to access the object hashes references directly. I keep explaining that this is bad practice, but he doesn't get it.
The problem with people like this is that they'll do other dumb things even if you don't allow them direct access to the hash. If they don't understand why it's a bad idea then they'll just find another "bad" way of solving the problem in my experience.