in reply to Module Design Review - Exporter::VA

Excellent! Here are my mostly minor comments.

s/sence/sense/, s/\$EXPORT/@EXPORT/, and you need to quote many of your hash keys since things like .foo and &foo are not barewords.

The function must return a reference to the proper type of thing, which is what will be placed in the caller's package.

I'd allow for callbacks to return undef to indicate that there is nothing to import so that modules can have non-importing options that use plain names.

Version strings are going away. You should not be using them in a new module.

        - tye

Replies are listed 'Best First'.
Re: (tye)Re: Module Design Review - Exporter::VA
by John M. Dlugosz (Monsignor) on Oct 29, 2002 at 15:48 UTC
    Thanks for your notes!

    you need to quote many of your hash keys since things like .foo and &foo are not barewords. Hmm, I guess that's true in general, since the hash keys look exactly like the symbol names! I wonder if that will be a source of annoying user errors, esp. since they may be defined and thus not caught as a warning in the parser!

    I wonder, would a different way of specifying these symbol names be better, or is the clarity of just naming them by what they are normally called make it worth using quotes?

    I'd allow for callbacks to return undef to indicate that there is nothing to import so that modules can have non-importing options that use plain names.

    I originally had that. Someone suggested standardizing on the dash for pragmatic imports. Doing so gives me more error checking. I'm on the fence now about allowing undef where its not expected.

    Version strings are going away. You should not be using them in a new module. So that begs several questions (1) if they are going away, what is taking their place? Some class type that has the same feature of relational operators implemented on it? (2) the use MODULE VERSION (LIST) syntax expects a version string. Is that going away too, or will it be improved, or what?

    By "going away" do you mean Perl6 or something sooner?

    —John

      I'd just quote them.

      You could make erroring out on undef in that case an option. If not, I would have to customize your module in order to use it with my existing module.

      A few months back I read that version strings had been declared a worse abomination than pseudo hashes and had already been deprecated. Searching for verification, that doesn't appear to be the case.

      Update: My existing module is Win32::TieRegistry and a typical use line looks like:

      my $Reg; use Win32::TieRegistry 0.20 ( TiedRef => \$Reg, Delimiter => "/", ArrayValues => 1, qw( :REG_ KEY_ALL_ACCESS ), );
      where options that take an argument are simply recognized by their value, not by having a leading "-". I'd already hoped to use the version argument to disable some default behavior that I had decided was a mistake, so your module would be a welcome aid. (:

              - tye
        I see. You have option/value pairs and constants imported in the normal manner complete with :TAG forms, but no functions!

        I already updated my design (back to) permitting pragmatic imports without a leading underscore. If you meant to write a callback and forgot to return something, you'll figure it out when the import doesn't import anything, so that's hardly a silent mysterious problem, and verbose mode will tell you exactly what really happened so you can find it easily enough.

        I'd be honored if you'd tinker with it before the official release, to make sure it does what you need for TieRegistry. I think that's a great example of something more exotic then the plain global function import situation.

        Tell me what your existing module does. I take it it doesn't use the Exporter module, but your own?

        Excuse the pun, but I couldn't resist - maybe that should be Win32::TyeRegistry? *ducks* Ok, ok, I'll be good now. :^)

        Makeshifts last the longest.