in reply to Private and Protected class methods
That said, though, there are times when slightly more robust enforcement of private and protected methods makes sense.
There are? I've yet to find myself in such a situation. This looks like a complete waste of time (human and CPU) to me! I much prefer to rely on coding standards and code reviews to avoid the kind of problems this is presumably attempting to address.
-sam
|
|---|
| Replies are listed 'Best First'. | |
|---|---|
|
Re^2: Private and Protected class methods (accidental)
by tye (Sage) on Sep 06, 2006 at 18:42 UTC | |
For me, the important distinction is accidental vs. intentional. Steps to discourage intentional misuse are mostly futile and are more than a waste, IMO. I usually find myself intentionally violating such restrictions because the author of the module lacked good design skills and making it a pain for me to overcome your stupid mistakes isn't a worth-while feature. And I don't want to make it a pain for other people to overcome my stupid mistakes. Trapping accidenal misuse can often be a good thing. Code reviews are a Good Thing™, but I don't want to rely on code reviews for enforcing complex restrictions when those can be trapped more reliably in an automated fashion. But I seriously doubt I'd make use of such a complex (my first impression) solution as proposed above. I'd need to see what real or likely accidental misuses this is intended to trap. My first impression of the above solution is that is appears broken anyway [it seems like it should be seeing if (caller(2)'s package)->can(caller(1)'s method name)]. So I probably don't understand the point of it. And I doubt I'd use inheritance to get this helper subroutine available to every class that wants to define protected subs. I'd probably just export it instead (into the "implementation" package that method code is compiled under so that I don't polute the object method namespace with things that aren't meant to be called as methods by users of my objects). As my impression of the idea of this sinks in, I start to think that I might use something similar to this in some cases. Removing inheritance and the polution of the namespace of object methods from its implementation makes me more comfortable with it. I'm not sure that the way @ISA is searched using can() is the best approach here. Then there is the complication that my methods are usually compiled into a package other than the one that objects get blessed into and I wouldn't want this check to incorrectly fire because of that. I'd have to do some digging on which packages are returned in which values from caller and some thinking about what really is supposed to be enforced here. No, I suspect that my OO "best practices" would break this code. I also suspect that stepping back and looking at the specific problem may lead to a solution other than "protected methods". - tye | [reply] |
by radiantmatrix (Parson) on Sep 06, 2006 at 19:07 UTC | |
My first impression of the above solution is that is appears broken anyway {it seems like it should be seeing if (caller(2)'s package)->can(caller(1)'s method name)}. So I probably don't understand the point of it. Actually, no (I have tested this). The subroutine name from caller is in the form Package::sub_name, where Package is always the name of the definining package. So, I check to see "who invoked the sub that called me?" and see if that package can sub_name. It works. It took me a while to get right, but it works. And I doubt I'd use inheritance to get this helper subroutine available to every class that wants to define protected subs. That's not exactly what I'm doing. This particular client has asked me to write an organization-wide base class (as in "all our classes will inherit from this"). This feature is being included there, so it must propogate via inheritance. That's not a design decision, it's a requirement. I'm not sure that the way @ISA is searched using can() is the best approach here. See, it's the only way I know of. I'd love to hear if there was another, better way. Care to elaborate? No, I suspect that my OO "best practices" would break this code. Can you be more specific? What do you do that would break this? How would it break? I also suspect that stepping back and looking at the specific problem may lead to a solution other than "protected methods". Like I said in my intro -- my personal feeling is to simply establish a coding standard that has a convention for naming protected and private methods, and let the code reviews, etc. catch things. That said, I also understand my client's point of view -- sometimes it's nice to kvetch about things like this (like the Perl::BestPractices module). The central point being that the ability to define private and protected methods is a requirement of my client, regardless of how I feel about them, so I need to find a workable solution that won't cause too many issues. You seem to suggest that the solution I'm proposing would cause some issues, but I would appreciate expansion of what those might be and what I have to consider to devise a work-around.
<–radiant.matrix–>
A collection of thoughts and links from the minds of geeks The Code that can be seen is not the true Code I haven't found a problem yet that can't be solved by a well-placed trebuchet | [reply] |
by tye (Sage) on Sep 06, 2006 at 20:26 UTC | |
But caller() is being used from PROTECTED(), and I'd think you'd want to know that whoever is calling the _protected_method() ->can( "_protected_method" ) so I'd think you'd want to grab the method name from caller(0) not caller(1) (after experimenting to figure out how many levels up caller($n) looks since the documentation isn't clear enough). But I freely admit that I don't grok the situtation and so could easily be mistaken (in particular, it looks like the fourth value from caller isn't as I expected). See, it's the only way I know of. I'd love to hear if there was another, better way. My first impression is that I'd check (calling package)->isa(called package). But I haven't thought any of the options through enough to declare any of them "better". Given your requirements, I'd name this method _PROTECTED() since it isn't meant to be used by your object users. See Re: How do YOU do OO in Perl? (constant offsets) and Re: OO - best way to have protected methods (packages) for some quick examples of my use of packages that might break your assumptions. In particular, consider a case where I've "exported" a sub into my method namespace. So caller will return the name of the package that the code was compiled into which isn't helpful here. Will caller return the name of the sub as it was defined or as it was called (since I might be calling My::Class::_protected_sub which got into My::Class by exporting Some::Other::Package::_other_name, for example)? Try this example:
- tye | [reply] [d/l] |
by radiantmatrix (Parson) on Sep 06, 2006 at 20:39 UTC | |
by Anonymous Monk on Sep 07, 2006 at 18:18 UTC | |
I don't see badly written modules as an excuse to violate encapsulation: I see it as a reason to fix the module in question. If you're really that much smarter than the module designer, and know so much about the internals of his design that you can reliably call functions that manage the system's internal state without any fear of ill side effects, why not just change the module to be what you want it to be? | [reply] |
by tye (Sage) on Sep 07, 2006 at 19:10 UTC | |
If you're really that much smarter than the module designer I never said I was smarter than anyone. Everyone makes mistakes. The mistakes I run into are usually simple mistakes not complex mistakes, hence "stupid mistakes". The most common being "nobody would ever need/want to do that", often probably the possibility was not even considered. This doesn't make the module designer stupid nor does it make me smarter. If I'd meant that I was smarter, then I wouldn't have mentioned my own stupid mistakes. But take offense if it helps you get through your day. (: know so much about the internals of his design that you can reliably call functions that manage the system's internal state without any fear of ill side effects Sorry, but most Perl modules aren't rocket surgery. I've got the source code. If I can't discern the inner workings of the parts of the module that I'm making use of fairly easily by reading the source code, then I'm likely to reconsider my decision to use the module. I rarely even have to grok the entire module contents. Even within a module, reasonable design principles mean that there are modular parts that are fairly isolated from each other. It is usually pretty easy to verify that doing X doesn't break things by just looking at every place that X touches. If traversing that tree of effects isn't a short trip, then it isn't a quick work-around and I'm not likely to use it. why not just change the module to be what you want it to be? Getting my work done with a quick work-around of some misfeature of a module can be very expeditious. Fixing the module's misfeature is usually quite a bit more complex. Often worse still, it creates a dependency problem. Making the fix of high enough quality that it is appropriate to put on CPAN is usually significantly more complex still. Then there is getting permission to distribute the fix. I discovered that a script was sucking up all of the available CPU and figured out that the problem was Term::ReadKey. I looked at the source and found that it quietly becomes a busy-loop if a certain compile-time test failed and, sure enough, that test fails on FreeBSD. I commented out that test, rebuilt, and the module works perfectly w/o being a CPU hog. Unfortunately, this was a diversion in a spare-time volunteer project and I regret that I didn't find the time to report the problem. Later, when the module author heard of the problem, they seemed a bit upset that I didn't send them the fix. I didn't have a fix. I had a work-around. Commenting out one test is trivial. Fixing the test so that it now works on the version of FreeBSD that I was using and doesn't work less well than the previous test on all prior releases of FreeBSD and all other operating systems that Perl runs on, is something that was way beyond my knowledge. I don't even have the knowledge that went into the selection of that test. So I'm not at all qualified (or "smart enough") to suggest a fix for it. If the work pans out and results in something that I'm going to need to do again or to release to others, then I need to clean things up and the quick work-around is no longer enough. Then I'd fix the module or look for an alternative. - tye | [reply] |