Let me give the background. I'm using Template Toolkit. Because I want to be able to write things like [% cgi.popup_menu(...).scalar %] and get reasonable output, I am using Template::Stash::Context. And then I decided that I'd like to also have it check to catch any use of unknown variables in the template.
If you look at the design then it is obvious that I need a different stash. And there is no stash on CPAN that does what I want. (Both strict checks and context.
Obviously I should just subclass it, right? Wrong. If you look in Template::Stash::Context you'll find checks in key places that check whether you're at the root by testing whether the reference type is __PACKAGE__. Which a subclassed object is not.
OK, can I cut and paste? I got that working, but then threw that solution away because I really don't like checking open source code with its licenses into proprietary codebases. Sure, I know what is OK to do, but I don't want anyone else to lose track. And I don't like giving lawyers heart attacks.
OK, let's just put a proxy class in front of it.
Nope. Didn't work. I didn't track it down, but I assume that it didn't work because it is sometimes passing stashes in calls to other stashes. There is more wrapping/unwrapping needed than I wanted to figure out.
Final solution?
# I do this because I don't want to copy somoeone else's copyrighted
# code into our codebase, and the class was not written to be easily
# subclassed.
no warnings 'redefine';
my $old_get = \&Template::Stash::Context::get;
*Template::Stash::Context::get = sub {
my ($self, $ident) = @_;
I'm unhappy that I don't give more context, but I'm able to store that in another place in my code so that my debugging messages have all the context I could need.
my $variable = ref($ident) ? $ident->[0] : $ident;
if (UNIVERSAL::isa($self, "HASH")
and not exists $self->{$variable}
and $variable ne "component"
and $variable ne "results"
) {
die "Variable $variable not found\n";
}
return $old_get->(@_);
};
That was far harder than it should have been. And it took me longer to come up with that answer than it should have. But that's what happens when I go back to thinking about Perl after spending all of my time writing SQL.
How ironic that you try to enable subclassing but you forbid polymorphism with that ghastly method-as-function silliness.
Re:Why Do You Hate Polymorphism?
bart on 2008-01-23T17:31:52
You mean it can be both a hash and not a hash? Wow.Re:Why Do You Hate Polymorphism?
chromatic on 2008-01-23T18:28:32
I can think of two ways this is possible in Perl 5. Both have been around for years.
Re:Why Do You Hate Polymorphism?
btilly on 2008-01-24T02:36:17
Read closer. There is no irony.
Polymorphism is already forbidden unless I want to rewrite a significant section of the existing code. I am therefore trying to find a way to get the behavior I want without doing that rewrite or else cutting and pasting.
The first two solutions that I tried (neither of which worked) tried to extend the existing code in such a way that they would not affect anyone else who wanted to extend the existing code using the same technique. In other words I was trying to make my approach as polymorphic as possible given the existing code.
Only after those approaches both failed did I consider injecting my code directly into that module. Given the structure of my application I am extremely confident that I only need one stash. And this behavior is what I want for my stash. So the non-polymorphism is not actually a problem.
However I tried to make it polymorphic. Really.Re:Why Do You Hate Polymorphism?
chromatic on 2008-01-24T03:53:38
Given the structure of my application I am extremely confident that I only need one stash. And this behavior is what I want for my stash. So the non-polymorphism is not actually a problem.I'm sure that's the same rationale why the original code is difficult to subclass; the author figured that one one would ever want to subclass it, so non-subclassibility is not actually a problem.
Re:Why Do You Hate Polymorphism?
btilly on 2008-01-24T04:10:26
There is a significant difference. I'm working on a code-base with a specific purpose that I'm the only developer on. This gives me a fair amount of insight into how it is likely to be modified in the future.
The module in question is used by a large number of people for many different things, and therefore the author should assume far less about how it is likely to be used in the future.
I also note that, even though I don't think polymorphism is important within my code base, I looked first for a solution that would preserve the possibility of polymorphism. The decision to not be polymorphic was not a case of, "This doesn't matter" but rather, "I've wasted far too much time trying find a solution that allows polymorphism, what can I come up with that works and lets me move on?" In other words that solution was a last choice, not a first choice.Re:Why Do You Hate Polymorphism?
chromatic on 2008-01-24T06:48:03
I looked first for a solution that would preserve the possibility of polymorphism.That solution is easy and it's shorter than what you have.
if (not eval { exists $self->{$variable} }... ) Re:Why Do You Hate Polymorphism?
btilly on 2008-01-24T20:38:44
You're complaining about the UNIVERSAL::isa?
That idiom appears within other code within Template::Stash::Context. Again, if you want to fix that, fixing it in my code is less important than changing it in the main module. Doubly so because it already can't be readily subclassed, which makes any worries about the best way to be polymorphic completely irrelevant.
That said, your fix has its own issues. Suppose that somewhere up the calling stack someone is using signals and a die handler. If that signal happens to come in while within your eval, then you've just swallowed the exception and they won't find out about the signal. You've just introduced a potential race condition. By the time you add in code to take care of that you've got more code than I have, and a greater likelyhood of bugs.
That is if you add it back in. As you just demonstrated, the odds of someone doing it correctly are fairly low. And for this code, needing to not swallow a DIE handler for a signal is far more likely to be a real issue than are the odds of UNIVERSAL::isa not being the right thing to call.
Which brings me to my final point. While I'm well aware of your quest to convince people to not call UNIVERSAL, and I'm aware of how you're willing to break working code to do so (I have sent you at least one patch in the past to avoid having you break other people's code), you have never convinced me that your goal is actually all that worthwhile. Therefore if satisfying you interferes with virtually any other desirable code criteria, I will not bother trying to satisfy you. Not because I particularly want to irritate you, but because I care about different things.Re:Why Do You Hate Polymorphism?
chromatic on 2008-01-25T00:18:52
Therefore if satisfying you interferes with virtually any other desirable code criteria, I will not bother trying to satisfy you. Not because I particularly want to irritate you, but because I care about different things.Glancing at the title of your journal entry here, one might reasonably assume that you care about things like Liskov substitution. Best of luck to you playing the odds in reliable software development.
(After all, it's not as if signal handling in Perl is at all reliable, whereas nothing in pure Perl requires breaking encapsulation, ever.)
Re:How about a patch?
btilly on 2008-01-24T02:47:02
Good idea.
Here is the necessary patch.
---/usr/lib/perl5/site_perl/5.8.6/i386-linux-thread-multi/Template/Stash/Context.p m 2007-04-27 10:56:05.000000000 -0700
+++ Context.pm 2008-01-23 18:40:56.000000000 -0800
@@ -134,6 +134,7 @@
%$params,
%$ROOT_OPS,
'_PARENT' => undef,
+ '_CLASS' => $class,
};
bless $self, $class;
@@ -422,7 +423,7 @@
$rootref = 'ARRAY';
$root = [$root];
}
- if ($rootref eq __PACKAGE__ || $rootref eq 'HASH') {
+ if ($rootref eq $self->{_CLASS} || $rootref eq 'HASH') {
# if $root is a regular HASH or a Template::Stash kinda HASH (the
# *real* root of everything). We first lookup the named key
@@ -607,7 +608,7 @@
return undef ## RETURN
unless $root and defined $item and $item !~/^[\._]/;
- if ($rootref eq 'HASH' || $rootref eq __PACKAGE__) {
+ if ($rootref eq 'HASH' || $rootref eq $self->{_CLASS}) {
# if ($item eq 'IMPORT' && UNIVERSAL::isa($value, 'HASH')) {
# # import hash entries into root hash
# @$root{ keys %$value } = values %$value;
I'll send it to Andy Wardley now.
