Stories
Slash Boxes
Comments
NOTE: use Perl; is on undef hiatus. You can read content, but you can't post it. More info will be forthcoming forthcomingly.

All the Perl that's Practical to Extract and Report

use Perl Log In

Log In

[ Create a new account ]

btilly (5037)

btilly
  (email not shown publicly)

Journal of btilly (5037)

Wednesday January 23, 2008
12:57 AM

Why make classes you can't subclass?

[ #35463 ]

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) = @_;

    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->(@_);
};

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.

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.

The Fine Print: The following comments are owned by whoever posted them. We are not responsible for them in any way.
 Full
 Abbreviated
 Hidden
More | Login | Reply
Loading... please wait.
  • How ironic that you try to enable subclassing but you forbid polymorphism with that ghastly method-as-function silliness.

    • You mean it can be both a hash and not a hash? Wow.
    • 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 app
      • 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.

        • 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
          • 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} } ... )
            • 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 ha
              • 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 noth

  • How about writing a patch to make it more subclassable and sending it to the TT mailing list? Andy seems to be in an active-development, patch-accepting phase at the moment.
    • Good idea.

      Here is the necessary patch.


      --- /usr/lib/perl5/site_perl/5.8.6/i386-linux-thread-multi/Template/Stash/Context.pm 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,
                }